Re: [PATCH 2/2 net v2] net: vxlan: fix nd_tbl NULL dereference when IPv6 is disabled

2026-03-02, 19:04:14 +0100, Fernando Fernandez Mancera wrote:
> On 3/2/26 6:44 PM, Fernando Fernandez Mancera wrote:
> > On 3/2/26 5:36 PM, Ido Schimmel wrote:
> > > On Mon, Mar 02, 2026 at 04:13:14PM +0100, Fernando Fernandez Mancera
> > > wrote:
> > > > On 3/2/26 3:43 PM, Sabrina Dubroca wrote:
> > > > > 2026-03-02, 15:08:47 +0100, Fernando Fernandez Mancera wrote:
> > > > > > When booting with the 'ipv6.disable=1' parameter, the nd_tbl is never
> > > > > > initialized because inet6_init() exits before ndisc_init() is called
> > > > > > which initializes it. If an IPv6 packet is injected into
> > > > > > the interface,
> > > > > > route_shortcircuit() is called and a NULL pointer
> > > > > > dereference happens on
> > > > > > neigh_lookup().
> > > > > 
> > > > > VXLAN can be built-in even when IPv6 is built as a module, and then
> > > > > ipv6_mod_enabled can't be used in VXLAN:
> > > > > 
> > > > > ld: drivers/net/vxlan/vxlan_core.o: in function `route_shortcircuit':
> > > > > drivers/net/vxlan/vxlan_core.c:2133:(.text+0x116e0):
> > > > > undefined reference to `ipv6_mod_enabled'
> > > > > 
> > > > > 
> > > > > GENEVE has a Kconfig tweak to prevent this since 094be0927ff3
> > > > > ("geneve: fix build with modular IPV6"):
> > > > > 
> > > > >       depends on IPV6 || !IPV6
> > > > > 
> > > > > but VXLAN doesn't.
> > > > > 
> > > > 
> > > > Ugh, good catch. Thanks Sabrina.
> > > > 
> > > > > But then I think we don't need to use ipv6_stub at all.
> > > > 
> > > > If we enforce "depends on IPV6 || IPV6=n" on VXLAN we could get rid of
> > > > ipv6_stub completely and use direct calls instead.
> > > 
> > > I just noticed that this is already enforced for the bridge, so maybe
> > > you can just remove ipv6_stub in patch #1?
> > > 
> > 
> > Yes, after Sabrina mentioned it that was what I planned to do. But on
> > VXLAN I want to avoid such patch for net tree as a fix. Instead I
> > propose to use the __in6_dev_get() check as neigh_reduce() does.

I'm not convinced __in6_dev_get() makes more sense. It's also pretty
much a random NULL check to me, and at least with checking nd_tbl
directly it's clearly related to the code that follows.

In neigh_reduce(), __in6_dev_get() seems to have been present from the
start, and back then in6_dev was used in the function. Now, it might
easily be removed by a clean-up patch that doesn't notice it's useful.


Should there be a helper ipv6_mod_available() (or a better name) in
net/ipv6/addrconf_core.c (so always available) to wrap those checks in
a way that doesn't rely on Kconfig magic nor non-obvious NULL checks?
(maybe even replacing ipv6_mod_enabled()?)

> > Later,
> > do a follow-up patch on net-next tree removing ipv6_stub and enforcing
> > "depends on IPV6 || IPV6=n".
> > 
> 
> FWIW; removing the ipv6_stub won't keep us from checking ipv6_mod_enabled()
> as ipv6.disable=1 can still be set at booting time and therefore nd_tbl will
> still be non-initialized because ndisc_init() and neigh_table_init() are
> never called.

Ok.

-- 
Sabrina