All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hangbin Liu <hangbin.liu@linux.dev>
To: Andrea Mayer <andrea.mayer@uniroma2.it>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	dsahern@kernel.org, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	idosch@nvidia.com, alex.aring@gmail.com, justin.iurman@gmail.com,
	bestswngs@gmail.com, stefano.salsano@uniroma2.it, xmei5@asu.edu,
	stable@vger.kernel.org
Subject: Re: [PATCH net v3] ipv6: rpl: fix NULL dereference of idev in ipv6_rpl_srh_rcv()
Date: Tue, 15 Sep 2026 14:03:02 +0800	[thread overview]
Message-ID: <aqjflqdySFG51YEa@fedora> (raw)
In-Reply-To: <20260817132644.2223-1-andrea.mayer@uniroma2.it>

Hi Andrea,

While reviewing the SRv6 code, I noticed you added an `idev` check in
`ipv6_rthdr_rcv()`. I’m wondering whether it makes sense to also halt
processing for `IPV6_SRCRT_TYPE_2` when `!idev`, given that IPv6 is
disabled on that device.

If so, could we drop the skb early at the entry point of `ipv6_rthdr_rcv()`?
The downside is that subsequent processing and `__IP6_INC_STATS()` would be
skipped.

BTW, `ipv6_rthdr_rcv()` handles both `IPV6_SRCRT_TYPE_3/4` entry points and
the full `IPV6_SRCRT_TYPE_2` processing path. There are four separate
`switch (hdr->type)` blocks inside this function. I plan to refactor it for
clearer logic. Do you think this is feasible, or unnecessary?

Thanks
Hangbin
On Mon, Aug 17, 2026 at 03:26:44PM +0200, Andrea Mayer wrote:
> ipv6_rpl_srh_rcv() dereferences idev from __in6_dev_get() without a NULL
> check when reading idev->cnf.rpl_seg_enabled.
> 
> When the device's MTU drops below IPV6_MIN_MTU, addrconf_ifdown() clears
> dev->ip6_ptr through RCU_INIT_POINTER(). A packet that passed the idev
> check in ip6_rcv_core() can then reach ipv6_rpl_srh_rcv() with
> dev->ip6_ptr already NULL.
> 
> Reproduced by flooding the receiving interface with ping6 traffic while
> flapping its MTU between 1500 and 1200:
> 
>  BUG: KASAN: null-ptr-deref in ipv6_rpl_srh_rcv+0xb3/0x1070
>  Read of size 4 at addr 00000000000006b4 by task ping6/394
> 
>  CPU: 2 UID: 0 PID: 394 Comm: ping6 Not tainted 7.2.0-rc7-micro-vm-dev-00095-g24ef02f934ee #240 PREEMPT(full)
>  Call Trace:
>   <IRQ>
>   kasan_report+0xc6/0x100
>   ipv6_rpl_srh_rcv+0xb3/0x1070
>   ip6_protocol_deliver_rcu+0x759/0x9a0
>   ip6_input_finish+0xa8/0x1b0
>   ip6_input+0xe1/0x490
>   ipv6_rcv+0x33d/0x460
>   __netif_receive_skb_one_core+0xd6/0x130
>   process_backlog+0x2cc/0xa00
>   __napi_poll.constprop.0+0x56/0x270
>   net_rx_action+0x327/0x730
>   handle_softirqs+0x11e/0x630
>   do_softirq+0xb3/0xf0
>   </IRQ>
> 
> Both ipv6_rpl_srh_rcv() and ipv6_srh_rcv() are called only from
> ipv6_rthdr_rcv(), which already has an idev lookup.
> 
> Fix the NULL dereference on the RPL path by checking idev in
> ipv6_rthdr_rcv(), before it calls either function. The callees take idev as
> an argument and no longer call __in6_dev_get(), so the packet is now
> dropped in one place, with SKB_DROP_REASON_IPV6DISABLED on both paths.
> 
> Fixes: 8610c7c6e3bd ("net: ipv6: add support for rpl sr exthdr")
> Cc: stable@vger.kernel.org
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> Tested-by: Xiang Mei <xmei5@asu.edu>
> ---
> v3:
>   - move the idev NULL check into ipv6_rthdr_rcv() and use the same drop
>     reason on the seg6 and RPL paths (David Ahern)
>   - pass idev to ipv6_srh_rcv() and ipv6_rpl_srh_rcv(), and check it for
>     NULL in ipv6_rthdr_rcv() only for the seg6 and RPL types
>   - add Xiang Mei's Tested-by tag
> v2: https://lore.kernel.org/netdev/20260518140630.24280-1-andrea.mayer@uniroma2.it/
>   - use SKB_DROP_REASON_IPV6DISABLED as drop reason (Eric Dumazet)
> v1: https://lore.kernel.org/netdev/20260428224816.11223-1-andrea.mayer@uniroma2.it/
> ---
>  net/ipv6/exthdrs.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 9c677eb1d1a6..51941ad656a3 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -368,23 +368,16 @@ static void seg6_update_csum(struct sk_buff *skb)
>  			   (__be32 *)addr);
>  }
>  
> -static int ipv6_srh_rcv(struct sk_buff *skb)
> +static int ipv6_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
>  {
>  	struct inet6_skb_parm *opt = IP6CB(skb);
>  	struct net *net = dev_net(skb->dev);
>  	struct ipv6_sr_hdr *hdr;
> -	struct inet6_dev *idev;
>  	struct in6_addr *addr;
>  	int accept_seg6;
>  
>  	hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb);
>  
> -	idev = __in6_dev_get(skb->dev);
> -	if (!idev) {
> -		kfree_skb(skb);
> -		return -1;
> -	}
> -
>  	accept_seg6 = min(READ_ONCE(net->ipv6.devconf_all->seg6_enabled),
>  			  READ_ONCE(idev->cnf.seg6_enabled));
>  
> @@ -485,12 +478,11 @@ static int ipv6_srh_rcv(struct sk_buff *skb)
>  	return -1;
>  }
>  
> -static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
> +static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
>  {
>  	struct ipv6_rpl_sr_hdr *hdr, *ohdr, *chdr;
>  	struct inet6_skb_parm *opt = IP6CB(skb);
>  	struct net *net = dev_net(skb->dev);
> -	struct inet6_dev *idev;
>  	struct ipv6hdr *oldhdr;
>  	unsigned int chdr_len;
>  	unsigned char *buf;
> @@ -499,8 +491,6 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
>  	u64 n = 0;
>  	u32 r;
>  
> -	idev = __in6_dev_get(skb->dev);
> -
>  	accept_rpl_seg = min(READ_ONCE(net->ipv6.devconf_all->rpl_seg_enabled),
>  			     READ_ONCE(idev->cnf.rpl_seg_enabled));
>  	if (!accept_rpl_seg) {
> @@ -689,10 +679,14 @@ static int ipv6_rthdr_rcv(struct sk_buff *skb)
>  	switch (hdr->type) {
>  	case IPV6_SRCRT_TYPE_4:
>  		/* segment routing */
> -		return ipv6_srh_rcv(skb);
> +		if (!idev)
> +			goto disabled;
> +		return ipv6_srh_rcv(skb, idev);
>  	case IPV6_SRCRT_TYPE_3:
>  		/* rpl segment routing */
> -		return ipv6_rpl_srh_rcv(skb);
> +		if (!idev)
> +			goto disabled;
> +		return ipv6_rpl_srh_rcv(skb, idev);
>  	default:
>  		break;
>  	}
> @@ -837,6 +831,10 @@ static int ipv6_rthdr_rcv(struct sk_buff *skb)
>  	icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
>  			  (&hdr->type) - skb_network_header(skb));
>  	return -1;
> +
> +disabled:
> +	kfree_skb_reason(skb, SKB_DROP_REASON_IPV6DISABLED);
> +	return -1;
>  }
>  
>  static const struct inet6_protocol rthdr_protocol = {
> -- 
> 2.43.0
> 

  parent reply	other threads:[~2026-09-15  6:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 13:26 [PATCH net v3] ipv6: rpl: fix NULL dereference of idev in ipv6_rpl_srh_rcv() Andrea Mayer
2026-08-20 12:28 ` Simon Horman
2026-08-20 19:30 ` patchwork-bot+netdevbpf
2026-09-15  6:03 ` Hangbin Liu [this message]
2026-09-15 12:24   ` Hangbin Liu
2026-09-17 17:41     ` Andrea Mayer
2026-09-18  1:13       ` Hangbin Liu

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=aqjflqdySFG51YEa@fedora \
    --to=hangbin.liu@linux.dev \
    --cc=alex.aring@gmail.com \
    --cc=andrea.mayer@uniroma2.it \
    --cc=bestswngs@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=justin.iurman@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=stefano.salsano@uniroma2.it \
    --cc=xmei5@asu.edu \
    /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.