bridge: Validate and clean up IPv6 neighbour suppression
Subsystems (4)
cover bridge: Validate and clean up IPv6 neighbour suppression
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
Signature · 1 line
2.54.0
PATCH 1/5 bridge: Use direct pointer in br_is_nd_neigh_msg()
Both callers of br_is_nd_neigh_msg() already call pskb_may_pull() to ensure sizeof(struct ipv6hdr) + sizeof(struct nd_msg) bytes are in the linear area before invoking this function. The skb_header_pointer() call and its fallback buffer are therefore unnecessary.
Replace skb_header_pointer() with a direct cast to ipv6_hdr(skb) + 1 and drop the now-unused 'msg' parameter and its corresponding stack buffer from all callers.
--- net/bridge/br_arp_nd_proxy.c | 9 ++------- net/bridge/br_device.c | 4 ++-- net/bridge/br_input.c | 4 ++-- net/bridge/br_private.h | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-)
net/bridge/br_arp_nd_proxy.c +2 -7
@@ -234,14 +234,9 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,#endif #if IS_ENABLED (CONFIG_IPV6 ) -struct nd_msg *br_is_nd_neigh_msg (const struct sk_buff *skb,struct nd_msg *msg) +struct nd_msg *br_is_nd_neigh_msg (const struct sk_buff *skb) { -struct nd_msg *m; - - m =skb_header_pointer (skb,skb_network_offset (skb) + -sizeof (struct ipv6hdr ),sizeof (*msg), msg); -if (!m) -return NULL ; +struct nd_msg *m = (struct nd_msg *)(ipv6_hdr (skb) +1 ); if (m->icmph.icmp6_code != 0 || (m->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION &&
net/bridge/br_device.c +2 -2
@@ -80,9 +80,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) pskb_may_pull(skb, sizeof(struct ipv6hdr) + sizeof(struct nd_msg)) && ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) { -struct nd_msg *msg, _msg; +struct nd_msg *msg; - msg =br_is_nd_neigh_msg (skb, &_msg); + msg =br_is_nd_neigh_msg (skb);if (msg)br_do_suppress_nd (skb, br, vid,NULL , msg); }
net/bridge/br_input.c +2 -2
@@ -176,9 +176,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb pskb_may_pull(skb, sizeof(struct ipv6hdr) + sizeof(struct nd_msg)) && ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) { -struct nd_msg *msg, _msg; +struct nd_msg *msg; - msg =br_is_nd_neigh_msg (skb, &_msg); + msg =br_is_nd_neigh_msg (skb);if (msg)br_do_suppress_nd (skb, br, vid, p, msg); }
net/bridge/br_private.h +1 -1
@@ -2365,7 +2365,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br, u16 vid, struct net_bridge_port *p);void br_do_suppress_nd (struct sk_buff *skb,struct net_bridge *br,u16 vid,struct net_bridge_port *p,struct nd_msg *msg); -struct nd_msg *br_is_nd_neigh_msg (const struct sk_buff *skb,struct nd_msg *m); +struct nd_msg *br_is_nd_neigh_msg (const struct sk_buff *skb);bool br_is_neigh_suppress_enabled (const struct net_bridge_port *p,u16 vid);bool br_is_neigh_forward_grat_enabled (const struct net_bridge_port *p,u16 vid);#endif
Signature · 1 line
2.54.0
Re: [PATCH net-next 1/5] bridge: Use direct pointer in br_is_nd_neigh_msg()
Quoted 9 lines · Danielle Ratson
Both callers of
br_is_nd_neigh_msg()already callpskb_may_pull()to ensure sizeof(struct ipv6hdr) +sizeof(struct nd_msg)bytes are in the linear area before invoking this function. Theskb_header_pointer()call and its fallback buffer are therefore unnecessary.Replace
skb_header_pointer()with a direct cast toipv6_hdr(skb)+ 1 and drop the now-unused 'msg' parameter and its corresponding stack buffer from all callers.
Quoted 9 lines
Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- net/bridge/br_arp_nd_proxy.c | 9 ++------- net/bridge/br_device.c | 4 ++-- net/bridge/br_input.c | 4 ++-- net/bridge/br_private.h | 2 +- 4 files changed, 7 insertions(+), 12 deletions(-)
PATCH 2/5 ipv6: ndisc: Add ndisc_check_ns_na() validation helper
In reply to Danielle RatsonAdd ndisc_check_ns_na(), a standalone NS/NA packet validator modeled after ipv6_mc_check_mld(). It performs the RFC 4861 section 7.1.1 (Neighbor Solicitation) and 7.1.2 (Neighbor Advertisement) mandatory checks that are relevant for software operating at the bridge level, where packets bypass the normal IPv6 stack path:
- Hop Limit must be 255 (packet was not forwarded by a router)
- ICMPv6 checksum is valid
- ICMP Code is 0
- ICMP length is at least 24 octets (sizeof(struct nd_msg))
- Target Address must not be a multicast address
- All included options have a length that is greater than zero
- NS/DAD: destination must be a solicited-node multicast address
- NS/DAD: no Source Link-Layer Address option when source is unspecified
- NA: Solicited flag must be 0 when IP Destination is multicast
On success the function sets the skb transport header and returns 0, matching the convention of ipv6_mc_check_mld().
--- include/net/ndisc.h | 2 + net/ipv6/Makefile | 2 +- net/ipv6/ndisc_snoop.c | 190 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 net/ipv6/ndisc_snoop.c
include/net/ndisc.h +2 -0
@@ -430,6 +430,8 @@ void ndisc_update(const struct net_device *dev, struct neighbour *neigh, const u8 *lladdr, u8 new, u32 flags, u8 icmp6_type, struct ndisc_options *ndopts); +int ndisc_check_ns_na (struct sk_buff *skb); +/* * IGMP */
net/ipv6/Makefile +1 -1
@@ -51,7 +51,7 @@ obj-$(subst m,y,$(CONFIG_IPV6)) += inet6_hashtables.o ifneq ($(CONFIG_IPV6),) obj-$(CONFIG_NET_UDP_TUNNEL) += ip6_udp_tunnel.o -obj-y += mcast_snoop.o +obj-y += mcast_snoop.o ndisc_snoop.o obj-$(CONFIG_TCP_AO) += tcp_ao.o endif
net/ipv6/ndisc_snoop.c new file +190 -0
190 lines not on this page · raw message
Signature · 1 line
2.54.0
Re: [PATCH net-next 2/5] ipv6: ndisc: Add ndisc_check_ns_na() validation helper
Quoted 19 lines · Danielle Ratson
Add
ndisc_check_ns_na(), a standalone NS/NA packet validator modeled afteripv6_mc_check_mld(). It performs the RFC 4861 section 7.1.1 (Neighbor Solicitation) and 7.1.2 (Neighbor Advertisement) mandatory checks that are relevant for software operating at the bridge level, where packets bypass the normal IPv6 stack path:- Hop Limit must be 255 (packet was not forwarded by a router)
- ICMPv6 checksum is valid
- ICMP Code is 0
- ICMP length is at least 24 octets (sizeof(struct nd_msg))
- Target Address must not be a multicast address
- All included options have a length that is greater than zero
- NS/DAD: destination must be a solicited-node multicast address
- NS/DAD: no Source Link-Layer Address option when source is unspecified
- NA: Solicited flag must be 0 when IP Destination is multicastOn success the function sets the skb transport header and returns 0, matching the convention of
ipv6_mc_check_mld().
Quoted 9 lines
Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- include/net/ndisc.h | 2 + net/ipv6/Makefile | 2 +- net/ipv6/ndisc_snoop.c | 190 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 1 deletion(-)create mode 100644
net/ipv6/ndisc_snoop.c
PATCH 3/5 bridge: Validate NS/NA messages using ndisc_check_ns_na()
In reply to Danielle RatsonThe bridge performs neighbor suppression by snooping NS/NA messages, but previously only checked the ICMPv6 type and code. This leaves it open to acting on malformed or spoofed packets that any RFC-compliant node should reject.
Wire br_is_nd_neigh_msg() into the new ndisc_check_ns_na() helper, which enforces the full RFC 4861 section 7.1.1/7.1.2 receive validation:
hop limit of 255, valid checksum, correct code, and type-specific rules (NS target not multicast; NA solicited flag clear for multicast destinations).
MLD messages are already validated by ipv6_mc_check_mld() before the bridge acts on them; this brings NS/NA to the same standard.
As a side effect, the skb parameter of br_is_nd_neigh_msg() changes from const to non-const, since ndisc_check_ns_na() may reallocate the skb head via pskb_may_pull() and sets the transport header. The returned pointer is now derived from skb_transport_header() rather than a direct cast.
--- net/bridge/br_arp_nd_proxy.c | 11 ++++------- net/bridge/br_private.h | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-)
net/bridge/br_arp_nd_proxy.c +4 -7
@@ -19,6 +19,7 @@#include <net/addrconf.h> #if IS_ENABLED (CONFIG_IPV6 )#include <net/ip6_checksum.h> +#include <net/ndisc.h> #endif #include "br_private.h"
@@ -234,16 +235,12 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,#endif #if IS_ENABLED (CONFIG_IPV6 ) -struct nd_msg *br_is_nd_neigh_msg (const struct sk_buff *skb) +struct nd_msg *br_is_nd_neigh_msg (struct sk_buff *skb) { -struct nd_msg *m = (struct nd_msg *)(ipv6_hdr (skb) +1 ); - -if (m->icmph.icmp6_code !=0 || - (m->icmph.icmp6_type !=NDISC_NEIGHBOUR_SOLICITATION && - m->icmph.icmp6_type !=NDISC_NEIGHBOUR_ADVERTISEMENT )) +if (ndisc_check_ns_na (skb))return NULL ; -return m; +return (struct nd_msg *)skb_transport_header (skb); } static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
net/bridge/br_private.h +1 -1
@@ -2365,7 +2365,7 @@ void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br, u16 vid, struct net_bridge_port *p);void br_do_suppress_nd (struct sk_buff *skb,struct net_bridge *br,u16 vid,struct net_bridge_port *p,struct nd_msg *msg); -struct nd_msg *br_is_nd_neigh_msg (const struct sk_buff *skb); +struct nd_msg *br_is_nd_neigh_msg (struct sk_buff *skb);bool br_is_neigh_suppress_enabled (const struct net_bridge_port *p,u16 vid);bool br_is_neigh_forward_grat_enabled (const struct net_bridge_port *p,u16 vid);#endif
Signature · 1 line
2.54.0
Re: [PATCH net-next 3/5] bridge: Validate NS/NA messages using ndisc_check_ns_na()
Quoted 19 lines · Danielle Ratson
The bridge performs neighbor suppression by snooping NS/NA messages, but previously only checked the ICMPv6 type and code. This leaves it open to acting on malformed or spoofed packets that any RFC-compliant node should reject.
Wire
br_is_nd_neigh_msg()into the newndisc_check_ns_na()helper, which enforces the full RFC 4861 section 7.1.1/7.1.2 receive validation:
hop limit of 255, valid checksum, correct code, and type-specific rules (NS target not multicast; NA solicited flag clear for multicast destinations).MLD messages are already validated by
ipv6_mc_check_mld()before the bridge acts on them; this brings NS/NA to the same standard.As a side effect, the skb parameter of
br_is_nd_neigh_msg()changes from const to non-const, sincendisc_check_ns_na()may reallocate the skb head viapskb_may_pull()and sets the transport header. The returned pointer is now derived fromskb_transport_header()rather than a direct cast.
Quoted 7 lines
Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- net/bridge/br_arp_nd_proxy.c | 11 ++++------- net/bridge/br_private.h | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-)
PATCH 4/5 bridge: Linearize skb once the ND message type is validated
In reply to Danielle Ratsonbr_nd_send() parses ND options from ns->opt[] and therefore needs the skb to be linear. Commit a01aee7cafc5 ("bridge: br_nd_send: linearize skb before parsing ND options") ensured that by linearizing inside br_nd_send() itself.
Move the linearization up into br_is_nd_neigh_msg(), right after ndisc_check_ns_na() has validated the message as an NS/NA. This makes a linear buffer a property of every recognized ND message, so that this and any future ND message handling operate on a linear skb and cannot reintroduce that class of bug by forgetting to linearize.
Since the skb is now linear by the time br_nd_send() runs, drop the linearization there and derive ns from the transport header set by ndisc_check_ns_na(), instead of recomputing it from the network header.
If linearization fails under memory pressure, br_is_nd_neigh_msg() returns NULL and the packet falls back to normal forwarding rather than being suppressed.
--- net/bridge/br_arp_nd_proxy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
net/bridge/br_arp_nd_proxy.c +5 -3
@@ -240,6 +240,9 @@ struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb)if (ndisc_check_ns_na (skb))return NULL ; +if (skb_linearize (skb)) +return NULL ; +return (struct nd_msg *)skb_transport_header (skb); }
@@ -259,7 +262,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,bool dad;u16 pvid; -if (!dev ||skb_linearize (request)) +if (!dev)return ; len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
@@ -276,8 +279,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,skb_set_mac_header (reply,0 ); daddr =eth_hdr (request)->h_source; - ns = (struct nd_msg *)(skb_network_header (request) + -sizeof (struct ipv6hdr )); + ns = (struct nd_msg *)skb_transport_header (request);/* Do we need option processing ? */ ns_olen = request->len - (skb_network_offset(request) +
Signature · 1 line
2.54.0
Nikolay Aleksandrov
maintainerRe: [PATCH net-next 4/5] bridge: Linearize skb once the ND message type is validated
Quoted 30 lines · Danielle Ratson
br_nd_send()parses ND options from ns->opt[] and therefore needs the skb to be linear. Commita01aee7cafc5("bridge:br_nd_send: linearize skb before parsing ND options") ensured that by linearizing insidebr_nd_send()itself.Move the linearization up into
br_is_nd_neigh_msg(), right afterndisc_check_ns_na()has validated the message as an NS/NA. This makes a linear buffer a property of every recognized ND message, so that this and any future ND message handling operate on a linear skb and cannot reintroduce that class of bug by forgetting to linearize.Since the skb is now linear by the time
br_nd_send()runs, drop the linearization there and derive ns from the transport header set byndisc_check_ns_na(), instead of recomputing it from the network header.If linearization fails under memory pressure,
br_is_nd_neigh_msg()returns NULL and the packet falls back to normal forwarding rather than being suppressed.Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- net/bridge/br_arp_nd_proxy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)net/bridge/br_arp_nd_proxy.c +0
-0
net/bridge/br_arp_nd_proxy.c struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb) · line 247
Quoted 8 lines
net/bridge/br_arp_nd_proxy.c +3
-0@@ -240,6 +240,9 @@ struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb)if (ndisc_check_ns_na (skb))return NULL ; +if (skb_linearize (skb)) +return NULL ; +return (struct nd_msg *)skb_transport_header (skb); }
This one is a bit weird - I wouldn't expect the check and validation to also linearize the skb. I'd rename this helper to show that it also linearizes the skb. Other than that the patch looks good.
Quoted 20 lines
net/bridge/br_arp_nd_proxy.c +2
-3@@ -259,7 +262,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,bool dad;u16 pvid; -if (!dev ||skb_linearize (request)) +if (!dev)return ; len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +@@ -276,8 +279,7 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,skb_set_mac_header (reply,0 ); daddr =eth_hdr (request)->h_source; - ns = (struct nd_msg *)(skb_network_header (request) + -sizeof (struct ipv6hdr )); + ns = (struct nd_msg *)skb_transport_header (request);/* Do we need option processing ? */ ns_olen = request->len - (skb_network_offset(request) +
RE: [PATCH net-next 4/5] bridge: Linearize skb once the ND message type is validated
Quoted 48 lines
-----Original Message-----
From: Nikolay Aleksandrov <[email protected]>
Sent: Monday, 20 July 2026 12:26
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 4/5] bridge: Linearize skb once the ND message type is validatedQuoted 19 lines · Danielle Ratson
br_nd_send()parses ND options from ns->opt[] and therefore needs the skb to be linear. Commita01aee7cafc5("bridge:br_nd_send: linearize skb before parsing ND options") ensured that by linearizing insidebr_nd_send()itself.Move the linearization up into
br_is_nd_neigh_msg(), right afterndisc_check_ns_na()has validated the message as an NS/NA. This makes a linear buffer a property of every recognized ND message, so that this and any future ND message handling operate on a linear skb and cannot reintroduce that class of bug by forgetting to linearize.Since the skb is now linear by the time
br_nd_send()runs, drop the linearization there and derive ns from the transport header set byndisc_check_ns_na(), instead of recomputing it from the network header.If linearization fails under memory pressure,
br_is_nd_neigh_msg()returns NULL and the packet falls back to normal forwarding rather than being suppressed.Quoted 8 lines
Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- net/bridge/br_arp_nd_proxy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)(quoted diff) +0
-0
b/net/bridge/br_arp_nd_proxy.cindex445c930ed59b..46779d9fad61100644
net/bridge/br_arp_nd_proxy.c struct nd_msg *br_is_nd_neigh_msg(struct sk_buff
Quoted 3 lines
net/bridge/br_arp_nd_proxy.c +0
-0@@ -240,6 +240,9 @@ struct nd_msg *br_is_nd_neigh_msg(struct sk_buff*skb)
Quoted 3 lines
net/bridge/br_arp_nd_proxy.c +0
-0if (ndisc_check_ns_na (skb))return NULL ;
Quoted 9 lines
net/bridge/br_arp_nd_proxy.c struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb) · line 247
Quoted 5 lines
net/bridge/br_arp_nd_proxy.c +3
-0+if (skb_linearize (skb)) +return NULL ; +return (struct nd_msg *)skb_transport_header (skb); }This one is a bit weird - I wouldn't expect the check and validation to also linearize the skb. I'd rename this helper to show that it also linearizes the skb. Other than that the patch looks good.
I agree it's not obvious from the name. But, I think it might be better to add a short comment than rename.
The name captures the intent, while linearizing is just prep for the option parsing in br_nd_send().
This also follows ipv6_mc_check_mld(), which is also a "check" that sets the transport header and documents its skb side effects in text rather than the name.
No other function here has a kdoc, so I'd keep it a plain comment:
/* Validate skb as an NS/NA and linearize it for br_nd_send()'s ND
* option parsing; returns the nd_msg, or NULL on failure. */
Would that work, or do you feel strongly about the rename?
Quoted 23 lines
net/bridge/br_arp_nd_proxy.c static void br_nd_send(struct net_bridge *br, struct
Quoted 3 lines
net/bridge/br_arp_nd_proxy.c +0
-0@@ -259,7 +262,7 @@ static void br_nd_send(struct net_bridge *br, struct
net_bridge_port*p,net/bridge/br_arp_nd_proxy.c static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
Quoted 9 lines
net/bridge/br_arp_nd_proxy.c +2
-1bool dad;u16 pvid; -if (!dev ||skb_linearize (request)) +if (!dev)return ; len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) + @@ -276,8 +279,7 @@ static void br_nd_send(struct net_bridge *br, struct
net_bridge_port*p,Quoted 9 lines
skb_set_mac_header(reply, 0); daddr = eth_hdr(request)->h_source;net/bridge/br_arp_nd_proxy.c +1
-2- ns = (struct nd_msg *)(skb_network_header (request) + -sizeof (struct ipv6hdr )); + ns = (struct nd_msg *)skb_transport_header (request);/* Do we need option processing ? */ ns_olen = request->len - (skb_network_offset(request) +
Nikolay Aleksandrov
maintainerRe: [PATCH net-next 4/5] bridge: Linearize skb once the ND message type is validated
Quoted 58 lines · Danielle Ratson
Quoted 48 lines
-----Original Message-----
From: Nikolay Aleksandrov <[email protected]>
Sent: Monday, 20 July 2026 12:26
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 4/5] bridge: Linearize skb once the ND message type is validatedQuoted 19 lines · Danielle Ratson
br_nd_send()parses ND options from ns->opt[] and therefore needs the skb to be linear. Commita01aee7cafc5("bridge:br_nd_send: linearize skb before parsing ND options") ensured that by linearizing insidebr_nd_send()itself.Move the linearization up into
br_is_nd_neigh_msg(), right afterndisc_check_ns_na()has validated the message as an NS/NA. This makes a linear buffer a property of every recognized ND message, so that this and any future ND message handling operate on a linear skb and cannot reintroduce that class of bug by forgetting to linearize.Since the skb is now linear by the time
br_nd_send()runs, drop the linearization there and derive ns from the transport header set byndisc_check_ns_na(), instead of recomputing it from the network header.If linearization fails under memory pressure,
br_is_nd_neigh_msg()returns NULL and the packet falls back to normal forwarding rather than being suppressed.Quoted 8 lines
Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- net/bridge/br_arp_nd_proxy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)(quoted diff) +0
-0
b/net/bridge/br_arp_nd_proxy.cindex445c930ed59b..46779d9fad61100644
net/bridge/br_arp_nd_proxy.c struct nd_msg *br_is_nd_neigh_msg(struct sk_buff
Quoted 3 lines
net/bridge/br_arp_nd_proxy.c +0
-0@@ -240,6 +240,9 @@ struct nd_msg *br_is_nd_neigh_msg(struct sk_buff*skb)
Quoted 3 lines
net/bridge/br_arp_nd_proxy.c +0
-0if (ndisc_check_ns_na (skb))return NULL ;Quoted 9 lines
net/bridge/br_arp_nd_proxy.c struct nd_msg *br_is_nd_neigh_msg(struct sk_buff *skb) · line 247
Quoted 5 lines
net/bridge/br_arp_nd_proxy.c +3
-0+if (skb_linearize (skb)) +return NULL ; +return (struct nd_msg *)skb_transport_header (skb); }This one is a bit weird - I wouldn't expect the check and validation to also linearize the skb. I'd rename this helper to show that it also linearizes the skb. Other than that the patch looks good.
Quoted 4 lines
I agree it's not obvious from the name. But, I think it might be better to add a short comment than rename.
The name captures the intent, while linearizing is just prep for the option parsing inbr_nd_send().
This also followsipv6_mc_check_mld(), which is also a "check" that sets the transport header and documents its skb side effects in text rather than the name.
Yeah, I'm not a fan of check functions having side effects at all, but it's not that big of a deal and more of a personal preference.
Quoted 8 lines
No other function here has a kdoc, so I'd keep it a plain comment:
/* Validate skb as an NS/NA and linearize it for br_nd_send()'s ND
* option parsing; returns thend_msg, or NULL on failure. */Would that work, or do you feel strongly about the rename?
That is ok.
Thanks,
Nik
Quoted 24 lines
Quoted 23 lines
net/bridge/br_arp_nd_proxy.c static void br_nd_send(struct net_bridge *br, struct
Quoted 3 lines
net/bridge/br_arp_nd_proxy.c +0
-0@@ -259,7 +262,7 @@ static void br_nd_send(struct net_bridge *br, struct
net_bridge_port*p,net/bridge/br_arp_nd_proxy.c static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p,
Quoted 9 lines
net/bridge/br_arp_nd_proxy.c +2
-1bool dad;u16 pvid; -if (!dev ||skb_linearize (request)) +if (!dev)return ; len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) + @@ -276,8 +279,7 @@ static void br_nd_send(struct net_bridge *br, struct
net_bridge_port*p,Quoted 9 lines
skb_set_mac_header(reply, 0); daddr = eth_hdr(request)->h_source;net/bridge/br_arp_nd_proxy.c +1
-2- ns = (struct nd_msg *)(skb_network_header (request) + -sizeof (struct ipv6hdr )); + ns = (struct nd_msg *)skb_transport_header (request);/* Do we need option processing ? */ ns_olen = request->len - (skb_network_offset(request) +
PATCH 5/5 bridge: Use ndisc_parse_options() to parse ND options in br_nd_send()
In reply to Danielle RatsonReplace the manual ND option parsing loop in br_nd_send() with ndisc_parse_options(), which provides proper validation and avoids the class of bugs that were fixed by commit 53fc685243bd ("bridge: Avoid infinite loop when suppressing NS messages with invalid options") and commit 850837965af1 ("bridge: br_nd_send: validate ND option lengths").
Use ndisc_opt_addr_data() to extract the source link-layer address from the parsed options, which correctly validates the option length for the underlying device type.
Export ndisc_parse_options() so that it can be resolved from the bridge when it is built as a module (CONFIG_BRIDGE=m); otherwise modpost fails with an undefined symbol.
--- net/bridge/br_arp_nd_proxy.c | 32 +++++++++++++++++--------------- net/ipv6/ndisc.c | 1 + 2 files changed, 18 insertions(+), 15 deletions(-)
net/bridge/br_arp_nd_proxy.c +17 -15
@@ -252,15 +252,16 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p, {struct net_device *dev = request->dev;struct net_bridge_vlan_group *vg; +struct ndisc_options ndopts;struct nd_msg *na, *ns;struct sk_buff *reply;struct ipv6hdr *pip6;int na_olen =8 ;/* opt hdr + ETH_ALEN for target */ int ns_olen; -int i, len;u8 *daddr;bool dad;u16 pvid; +int len;if (!dev)return ;
@@ -281,20 +282,21 @@ static void br_nd_send(struct net_bridge *br, struct net_bridge_port *p, daddr =eth_hdr (request)->h_source; ns = (struct nd_msg *)skb_transport_header (request); -/* Do we need option processing ? */ - ns_olen = request->len - (skb_network_offset (request) + -sizeof (struct ipv6hdr )) -sizeof (*ns); -for (i =0 ; i < ns_olen -1 ; i += (ns->opt[i +1 ] <<3 )) { -if (!ns->opt[i +1 ] || i + (ns->opt[i +1 ] <<3 ) > ns_olen) { -kfree_skb (reply); -return ; - } -if (ns->opt[i] ==ND_OPT_SOURCE_LL_ADDR ) { -if ((ns->opt[i +1 ] <<3 ) >= -sizeof (struct nd_opt_hdr ) +ETH_ALEN ) - daddr = ns->opt + i +sizeof (struct nd_opt_hdr ); -break ; - } +/* Derive the option length from the IPv6 payload length so that any +* trailing L2 padding in the skb is not parsed as ND options. +*/ + ns_olen =ntohs (ipv6_hdr (request)->payload_len) -sizeof (*ns); +if (!ndisc_parse_options (dev, ns->opt, ns_olen, &ndopts)) { +kfree_skb (reply); +return ; + } + +if (ndopts.nd_opts_src_lladdr) { +u8 *lladdr; + + lladdr =ndisc_opt_addr_data (ndopts.nd_opts_src_lladdr, dev); +if (lladdr) + daddr = lladdr; } dad =ipv6_addr_any (&ipv6_hdr (request)->saddr);
net/ipv6/ndisc.c +1 -0
@@ -283,6 +283,7 @@ struct ndisc_options *ndisc_parse_options(const struct net_device *dev, }return ndopts; } +EXPORT_SYMBOL (ndisc_parse_options);int ndisc_mc_map (const struct in6_addr *addr,char *buf,struct net_device *dev,int dir) {
Signature · 1 line
2.54.0
Re: [PATCH net-next 5/5] bridge: Use ndisc_parse_options() to parse ND options in br_nd_send()
Quoted 14 lines · Danielle Ratson
Replace the manual ND option parsing loop in
br_nd_send()withndisc_parse_options(), which provides proper validation and avoids the class of bugs that were fixed by commit53fc685243bd("bridge: Avoid infinite loop when suppressing NS messages with invalid options") and commit850837965af1("bridge:br_nd_send: validate ND option lengths").Use
ndisc_opt_addr_data()to extract the source link-layer address from the parsed options, which correctly validates the option length for the underlying device type.Export
ndisc_parse_options()so that it can be resolved from the bridge when it is built as a module (CONFIG_BRIDGE=m); otherwise modpost fails with an undefined symbol.
Quoted 7 lines
Reviewed-by Petr Machata <[email protected]>Signed-off-by Danielle Ratson <[email protected]>--- net/bridge/br_arp_nd_proxy.c | 32 +++++++++++++++++--------------- net/ipv6/ndisc.c | 1 + 2 files changed, 18 insertions(+), 15 deletions(-)
Nikolay Aleksandrov
maintainerQuoted 46 lines · Danielle Ratson
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 afteripv6_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()withndisc_parse_options()andndisc_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-unnecessaryskb_header_pointer()fallback frombr_is_nd_neigh_msg().No functional change is intended for well-formed packets.
Patchset overview:
Patch #1: drop theskb_header_pointer()fallback. Patches #2-#3: addndisc_check_ns_na()and validate NS/NA with it. Patch #4: linearize once the ND message type is validated. Patch #5: parse options viandisc_parse_options().Danielle Ratson (5):
bridge: Use direct pointer inbr_is_nd_neigh_msg()
ipv6: ndisc: Addndisc_check_ns_na()validation helper
bridge: Validate NS/NA messages usingndisc_check_ns_na()
bridge: Linearize skb once the ND message type is validated
bridge: Usendisc_parse_options()to parse ND options inbr_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(-)
Quoted 2 lines
create mode 100644
net/ipv6/ndisc_snoop.c
Nice set, but I'm curious - any reason not to use EXPORT_SYMBOL_GPL() instead?
Cheers,
Nik
Quoted 59 lines
-----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 suppressionQuoted 38 lines · Danielle Ratson
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 afteripv6_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()withndisc_parse_options()andndisc_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-unnecessaryskb_header_pointer()fallback frombr_is_nd_neigh_msg().No functional change is intended for well-formed packets.
Patchset overview:
Patch #1: drop theskb_header_pointer()fallback. Patches #2-#3: addndisc_check_ns_na()and validate NS/NA with it. Patch #4: linearize once the ND message type is validated. Patch #5: parse options viandisc_parse_options().Danielle Ratson (5):
bridge: Use direct pointer inbr_is_nd_neigh_msg()
ipv6: ndisc: Addndisc_check_ns_na()validation helper
bridge: Validate NS/NA messages usingndisc_check_ns_na()
bridge: Linearize skb once the ND message type is validated
bridge: Usendisc_parse_options()to parse ND options inbr_nd_send()Quoted 8 lines
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
Quoted 8 lines
+++++++++++++++++++++++++++++++++++
Quoted 3 lines
8 files changed, 224 insertions(+), 35 deletions(-)create mode 100644
net/ipv6/ndisc_snoop.cNice 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.
Quoted 2 lines
Cheers,
Nik
Nikolay Aleksandrov
maintainerQuoted 64 lines · Danielle Ratson
Quoted 64 lines
-----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 suppressionQuoted 38 lines · Danielle Ratson
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 afteripv6_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()withndisc_parse_options()andndisc_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-unnecessaryskb_header_pointer()fallback frombr_is_nd_neigh_msg().No functional change is intended for well-formed packets.
Patchset overview:
Patch #1: drop theskb_header_pointer()fallback. Patches #2-#3: addndisc_check_ns_na()and validate NS/NA with it. Patch #4: linearize once the ND message type is validated. Patch #5: parse options viandisc_parse_options().Danielle Ratson (5):
bridge: Use direct pointer inbr_is_nd_neigh_msg()
ipv6: ndisc: Addndisc_check_ns_na()validation helper
bridge: Validate NS/NA messages usingndisc_check_ns_na()
bridge: Linearize skb once the ND message type is validated
bridge: Usendisc_parse_options()to parse ND options inbr_nd_send()Quoted 8 lines
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+++++++++++++++++++++++++++++++++++
Quoted 3 lines
8 files changed, 224 insertions(+), 35 deletions(-)create mode 100644
net/ipv6/ndisc_snoop.c
Quoted 8 lines
Quoted 3 lines
Nice set, but I'm curious - any reason not to use
EXPORT_SYMBOL_GPL()instead?I guess you are referring to
ndisc_parse_optionsandndisc_check_ns_na?
The reason is basically that other sibling functions usedEXPORT_SYMBOL()rather thanEXPORT_SYMBOL_GPL()(ndisc_send_skb,ndisc_ns_create, and thendisc_check_ns_na()equivalent in mld,ipv6_mc_check_mld()). However,ndisc_send_na()for example is newer and usesEXPORT_SYMBOL_GPL(). So it might make sense to use that inndisc_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
Quoted 2 lines
Quoted 2 lines
Cheers,
Nik
Quoted 73 lines · Nikolay Aleksandrov
Quoted 64 lines · Danielle Ratson
Quoted 64 lines
-----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 suppressionQuoted 38 lines · Danielle Ratson
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 afteripv6_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()withndisc_parse_options()andndisc_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-unnecessaryskb_header_pointer()fallback frombr_is_nd_neigh_msg().No functional change is intended for well-formed packets.
Patchset overview:
Patch #1: drop theskb_header_pointer()fallback. Patches #2-#3: addndisc_check_ns_na()and validate NS/NA with it. Patch #4: linearize once the ND message type is validated. Patch #5: parse options viandisc_parse_options().Danielle Ratson (5):
bridge: Use direct pointer inbr_is_nd_neigh_msg()
ipv6: ndisc: Addndisc_check_ns_na()validation helper
bridge: Validate NS/NA messages usingndisc_check_ns_na()
bridge: Linearize skb once the ND message type is validated
bridge: Usendisc_parse_options()to parse ND options inbr_nd_send()Quoted 8 lines
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+++++++++++++++++++++++++++++++++++
Quoted 3 lines
8 files changed, 224 insertions(+), 35 deletions(-)create mode 100644
net/ipv6/ndisc_snoop.cQuoted 7 lines
Quoted 3 lines
Nice set, but I'm curious - any reason not to use
EXPORT_SYMBOL_GPL()instead?I guess you are referring to
ndisc_parse_optionsandndisc_check_ns_na?
The reason is basically that other sibling functions usedEXPORT_SYMBOL()rather thanEXPORT_SYMBOL_GPL()(ndisc_send_skb,ndisc_ns_create, and thendisc_check_ns_na()equivalent in mld,ipv6_mc_check_mld()). However,ndisc_send_na()for example is newer and usesEXPORT_SYMBOL_GPL(). So it might make sense to use that inndisc_parse_options(), if you prefer.
Quoted 4 lines
Yep, I was referring to those newly exported symbols.
Personally I'd always go for the_GPLvariant 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.