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

On 3/3/26 12:02 AM, Sabrina Dubroca wrote:
> 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.
> 

Fair enough, let's just stick to the nd_tbl NULL check for now on VXLAN. 
I will send a patch on net-next tree once this is merged to enforce 
"depends on IPV6 || IPV6=n" and remove ipv6_stub from there.

I hope that sounds good to you all.

> 
> 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()?)
> 

Maybe yes, but not sure if it is worth it. As discussed here [1] 
dropping IPv6 as a module seems like a good idea. Anyway, let's discuss 
that topic there!

[1] https://lore.kernel.org/netdev/[email protected]/

Thanks,
Fernando.

>>> 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.
>