mirror of https://lore.kernel.org/lkml/
 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: Fri, 18 Sep 2026 09:13:47 +0800	[thread overview]
Message-ID: <aqyQS7knJ2SwKxPG@fedora> (raw)
In-Reply-To: <20260917194128.c078117a13eca6f3c97e3dde@uniroma2.it>

Hi Andrea,
On Thu, Sep 17, 2026 at 07:41:28PM +0200, Andrea Mayer wrote:
> On Tue, 15 Sep 2026 20:24:22 +0800
> Hangbin Liu <hangbin.liu@linux.dev> wrote:
> 
> > On Tue, Sep 15, 2026 at 02:03:12PM +0800, Hangbin Liu wrote:
> > > 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.
> > 
> > Oh, another behavior change. At present, if !idev, we can still process the
> > IPV6_SRCRT_TYPE_2 message. And the process will be skipped if we return early.
> > I'm not sure if this kind of change is acceptable or not in kernel.
> > 
> > But on the other hand, is it correct to accept and forward SRCRT Type 2
> > packets on device that IPv6 is disabled?
> > 
> > Thanks
> > Hangbin
> 
> Hi Hangbin,
> 
> One case is when IPv6 is already disabled as the packet arrives:
> ip6_rcv_core() drops the packet before ipv6_rthdr_rcv().
> The other is the transient window: dev->ip6_ptr can be cleared after
> that check, and ipv6_rthdr_rcv() can then find idev NULL.
> In that window the TYPE_2 path can run without idev. ipv6_srh_rcv() and
> ipv6_rpl_srh_rcv() cannot: they read idev->cnf. That is why the check
> sits before those two calls.

Thanks for your reply. So in fact, if IPv6 is disabled on an interface,
*all* SRCRT types get dropped in ip6_rcv_core(). The idev check inside
ipv6_rthdr_rcv() exists to close this transient window.

> 
> On the counters: __IP6_INC_STATS() goes through _DEVINC(), which skips
> the per-device bump when idev is NULL and still increments the per-net one.
> Stopping TYPE_2 in that window as well is a behaviour change, and I think
> it would need a reason of its own.

My understanding is that handling TYPE_2 within that window is also mismatched
with ip6_rcv_core(), theoretically it should not be processed.

However, since there is no harm for TYPE_2 with the current code. We can
retain the existing behavior unless someone reports issues, or until I have
time to restructure this function.

Thanks
Hangbin

      reply	other threads:[~2026-09-18  1:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 13:26 Andrea Mayer
2026-08-20 12:28 ` Simon Horman
2026-08-20 19:30 ` patchwork-bot+netdevbpf
2026-09-15  6:03 ` Hangbin Liu
2026-09-15 12:24   ` Hangbin Liu
2026-09-17 17:41     ` Andrea Mayer
2026-09-18  1:13       ` Hangbin Liu [this message]

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=aqyQS7knJ2SwKxPG@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®