Nikolay Aleksandrov <[email protected]> writes:
> 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. :) Yeah, I missed this during the review. I'm in favor of GPL as well.
