Re: [PATCH net-next 0/5] bridge: Validate and clean up IPv6 neighbour suppression

Nikolay Aleksandrov <[email protected]> Thu, 23 Jul 2026 12:20:40 +0300
Newsgroups gmane.linux.network.bridge,gmane.linux.network,gmane.linux.kernel
Message-ID <[email protected]>
On 23/07/2026 09:40, Danielle Ratson wrote:
>> -----Original Message-----
>> From: Nikolay Aleksandrov <[email protected]>
>> Sent: Monday, 20 July 2026 12:31
>> To: Danielle Ratson <[email protected]>; [email protected]
>> Cc: [email protected]; Ido Schimmel <[email protected]>;
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected]; Petr Machata
>> <[email protected]>; [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH net-next 0/5] bridge: Validate and clean up IPv6
>> neighbour suppression
>>
>> On 19/07/2026 16:34, Danielle Ratson wrote:
>>> The bridge implements IPv6 neighbour suppression by snooping Neighbour
>>> Solicitation and Neighbour Advertisement messages, but it previously
>>> only checked the ICMPv6 type and code before acting on them. This
>>> leaves it open to acting on malformed or spoofed packets that any RFC
>>> 4861 compliant node should reject, and the option parsing in
>>> br_nd_send() open-codes a loop that has historically been a source of bugs.
>>>
>>> This series hardens and cleans up that path:
>>>
>>> Add ndisc_check_ns_na(), a standalone NS/NA validator modeled after
>>> ipv6_mc_check_mld(), implementing the RFC 4861 section 7.1.1 / 7.1.2
>>> mandatory receive checks (hop limit, checksum, code, length, target
>>> and option validation). Wire the bridge into it so NS/NA messages are
>>> validated to the same standard MLD already enjoys.
>>>
>>> Replace the manual ND option parsing loop in br_nd_send() with
>>> ndisc_parse_options() and ndisc_opt_addr_data(), and linearize the skb
>>> once it has been validated as an NS/NA message so that this and any
>>> future ND message handling operate on a linear buffer. The first patch
>>> is a small preparatory cleanup that drops the now-unnecessary
>>> skb_header_pointer() fallback from br_is_nd_neigh_msg().
>>>
>>> No functional change is intended for well-formed packets.
>>>
>>> Patchset overview:
>>> Patch #1: drop the skb_header_pointer() fallback.
>>> Patches #2-#3: add ndisc_check_ns_na() and validate NS/NA with it.
>>> Patch #4: linearize once the ND message type is validated.
>>> Patch #5: parse options via ndisc_parse_options().
>>>
>>> Danielle Ratson (5):
>>>     bridge: Use direct pointer in br_is_nd_neigh_msg()
>>>     ipv6: ndisc: Add ndisc_check_ns_na() validation helper
>>>     bridge: Validate NS/NA messages using ndisc_check_ns_na()
>>>     bridge: Linearize skb once the ND message type is validated
>>>     bridge: Use ndisc_parse_options() to parse ND options in
>>> br_nd_send()
>>>
>>>    include/net/ndisc.h          |   2 +
>>>    net/bridge/br_arp_nd_proxy.c |  54 +++++-----
>>>    net/bridge/br_device.c       |   4 +-
>>>    net/bridge/br_input.c        |   4 +-
>>>    net/bridge/br_private.h      |   2 +-
>>>    net/ipv6/Makefile            |   2 +-
>>>    net/ipv6/ndisc.c             |   1 +
>>>    net/ipv6/ndisc_snoop.c       | 190
>> +++++++++++++++++++++++++++++++++++
>>>    8 files changed, 224 insertions(+), 35 deletions(-)
>>>    create mode 100644 net/ipv6/ndisc_snoop.c
>>>
>>
>> Nice set, but I'm curious - any reason not to use EXPORT_SYMBOL_GPL()
>> instead?
>>
> 
> I guess you are referring to ndisc_parse_options and ndisc_check_ns_na?
> The reason is basically that other sibling functions used EXPORT_SYMBOL() rather than EXPORT_SYMBOL_GPL() (ndisc_send_skb, ndisc_ns_create, and the ndisc_check_ns_na() equivalent in mld, ipv6_mc_check_mld()).
> However, ndisc_send_na() for example is newer and uses EXPORT_SYMBOL_GPL(). So it might make sense to use that in ndisc_parse_options(), if you prefer.
> 

Yep, I was referring to those newly exported symbols.
Personally I'd always go for the _GPL variant when possible, but it's not a
strong preference, more a personal one. I was just curious if there was some
particular reason not to be. :)

Thanks,
  Nik

>> Cheers,
>>    Nik