All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Simon Horman <horms@kernel.org>
Cc: netdev@vger.kernel.org, davem@davemloft.net, laforge@osmocom.org,
	pespin@sysmocom.de, osmith@sysmocom.de, kuba@kernel.org,
	pabeni@redhat.com, edumazet@google.com, fw@strlen.de
Subject: Re: [PATCH net-next 04/12] gtp: add IPv6 support
Date: Thu, 2 May 2024 12:38:04 +0200	[thread overview]
Message-ID: <ZjNtDEJFDgSjWanp@calendula> (raw)
In-Reply-To: <20240426204101.GE516117@kernel.org>

Hi Simon,

On Fri, Apr 26, 2024 at 09:41:01PM +0100, Simon Horman wrote:
> On Thu, Apr 25, 2024 at 12:51:30PM +0200, Pablo Neira Ayuso wrote:
[...]
> > @@ -131,6 +134,11 @@ static inline u32 ipv4_hashfn(__be32 ip)
> >  	return jhash_1word((__force u32)ip, gtp_h_initval);
> >  }
> >  
> > +static inline u32 ipv6_hashfn(const struct in6_addr *ip6)
> > +{
> > +	return jhash(ip6, sizeof(*ip6), gtp_h_initval);
> > +}
> > +
> 
> Hi Pablo,
> 
> I'm would naively expect that the compiler can work out if this needs to
> be inline.

I will remove inline, I saw the warnings from patchwork on this too
after my v2.

> >  /* Resolve a PDP context structure based on the 64bit TID. */
> >  static struct pdp_ctx *gtp0_pdp_find(struct gtp_dev *gtp, u64 tid)
> >  {
> 
> ...
> 
> > @@ -878,6 +951,20 @@ static inline void gtp_set_pktinfo_ipv4(struct gtp_pktinfo *pktinfo,
> >  	pktinfo->dev	= dev;
> >  }
> >  
> > +static inline void gtp_set_pktinfo_ipv6(struct gtp_pktinfo *pktinfo,
> > +					struct sock *sk, struct ipv6hdr *ip6h,
> > +					struct pdp_ctx *pctx, struct rt6_info *rt6,
> > +					struct flowi6 *fl6,
> > +					struct net_device *dev)
> > +{
> > +	pktinfo->sk	= sk;
> > +	pktinfo->ip6h	= ip6h;
> > +	pktinfo->pctx	= pctx;
> > +	pktinfo->rt6	= rt6;
> > +	pktinfo->fl6	= *fl6;
> > +	pktinfo->dev	= dev;
> > +}
> 
> Here too.

OK.

> > @@ -1441,7 +1736,14 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
> >  		if (!pctx)
> >  			pctx = pctx_tid;
> >  
> > -		ipv4_pdp_fill(pctx, info);
> > +		switch (pctx->af) {
> > +		case AF_INET:
> > +			ipv4_pdp_fill(pctx, info);
> > +			break;
> > +		case AF_INET6:
> > +			ipv6_pdp_fill(pctx, info);
> > +			break;
> > +		}
> >  
> >  		if (pctx->gtp_version == GTP_V0)
> >  			netdev_dbg(dev, "GTPv0-U: update tunnel id = %llx (pdp %p)\n",
> 
> The code just before the following hunk is:
> 
> 	pctx = kmalloc(sizeof(*pctx), GFP_ATOMIC);
> 	if (pctx == NULL)
> 		return ERR_PTR(-ENOMEM);
> 
> 
> > @@ -1461,7 +1763,24 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
> >  	sock_hold(sk);
> >  	pctx->sk = sk;
> >  	pctx->dev = gtp->dev;
> > -	ipv4_pdp_fill(pctx, info);
> > +	pctx->af = family;
> > +
> > +	switch (pctx->af) {
> > +	case AF_INET:
> > +		if (!info->attrs[GTPA_MS_ADDRESS] ||
> > +		    !info->attrs[GTPA_PEER_ADDRESS])
> > +			return ERR_PTR(-EINVAL);
> 
> So this appears to leak pctx.

Good catch.

> > +
> > +		ipv4_pdp_fill(pctx, info);
> > +		break;
> > +	case AF_INET6:
> > +		if (!info->attrs[GTPA_MS_ADDR6] ||
> > +		    !info->attrs[GTPA_PEER_ADDR6])
> > +			return ERR_PTR(-EINVAL);
> 
> Likewise here.
> 
> Flagged by Smatch.

Thanks Simon.

  reply	other threads:[~2024-05-02 10:38 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-25 10:51 [PATCH net-next 00/12] gtp updates for net-next (v2) Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 01/12] gtp: remove useless initialization Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 02/12] gtp: properly parse extension headers Pablo Neira Ayuso
2024-04-26 20:28   ` Simon Horman
2024-05-02 10:36     ` Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 03/12] gtp: prepare for IPv6 support Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 04/12] gtp: add " Pablo Neira Ayuso
2024-04-26 20:41   ` Simon Horman
2024-05-02 10:38     ` Pablo Neira Ayuso [this message]
2024-04-25 10:51 ` [PATCH net-next 05/12] gtp: use IPv6 address /64 prefix for UE/MS Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 06/12] gtp: pass up link local traffic to userspace socket Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 07/12] gtp: move debugging to skbuff build helper function Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 08/12] gtp: remove IPv4 and IPv6 header from context object Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 09/12] gtp: add helper function to build GTP packets from an IPv4 packet Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 10/12] gtp: add helper function to build GTP packets from an IPv6 packet Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 11/12] gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP Pablo Neira Ayuso
2024-04-25 10:51 ` [PATCH net-next 12/12] gtp: identify tunnel via GTP device + GTP version + TEID + family Pablo Neira Ayuso
2024-04-26  2:29 ` [PATCH net-next 00/12] gtp updates for net-next (v2) Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2024-04-23 22:39 [PATCH net-next 00/12] GTP driver updates for net-next Pablo Neira Ayuso
2024-04-23 22:39 ` [PATCH net-next 04/12] gtp: add IPv6 support Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZjNtDEJFDgSjWanp@calendula \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=laforge@osmocom.org \
    --cc=netdev@vger.kernel.org \
    --cc=osmith@sysmocom.de \
    --cc=pabeni@redhat.com \
    --cc=pespin@sysmocom.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.