RE: [PATCH net-next v2 1/5] bridge: Use direct pointer in br_is_nd_neigh_msg()
Danielle Ratson <[email protected]>
| Newsgroups | dev.linux.lists.bridge,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <SJ2PR12MB900899F8686070DFA256F52BD8D32@SJ2PR12MB9008.namprd12.prod.outlook.com> |
> -----Original Message----- > From: Danielle Ratson <[email protected]> > Sent: Monday, 3 August 2026 14:25 > To: [email protected] > Cc: [email protected]; Ido Schimmel <[email protected]>; > [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]; Danielle Ratson > <[email protected]> > Subject: [PATCH net-next v2 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. > > Reviewed-by: Petr Machata <[email protected]> > Acked-by: Nikolay Aleksandrov <[email protected]> > Signed-off-by: Danielle Ratson <[email protected]> > --- Sashiko wrote: " > diff --git a/net/bridge/br_arp_nd_proxy.c b/net/bridge/br_arp_nd_proxy.c > index 23eb6931a2b4a..db08c3272001f 100644 > --- a/net/bridge/br_arp_nd_proxy.c > +++ b/net/bridge/br_arp_nd_proxy.c This is a pre-existing issue, but can br_do_proxy_suppress_arp() also suffer from an out-of-bounds read when skb_network_offset(skb) > 0? In br_do_proxy_suppress_arp(), pskb_may_pull() is used to validate arp_hdr_len(dev) relative to skb->data: if ((dev->flags & IFF_NOARP) || !pskb_may_pull(skb, arp_hdr_len(dev))) return; parp = arp_hdr(skb); Since arp_hdr(skb) resolves to skb_network_header(skb), accessing the arp payload reads beyond the validated linear area if the network offset is greater than 0. This could happen if a bpf program at traffic control egress uses bpf_skb_adjust_room to shift the network header relative to skb->data before bridge processing. " Pre-existing issue, not in the scope. " > @@ -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); Does this cast introduce an out-of-bounds read if skb_network_offset(skb) > 0? The callers use pskb_may_pull() to validate the length relative to skb->data. However, ipv6_hdr(skb) resolves to skb_network_header(skb), which is skb->data + skb_network_offset(skb). If a packet is sent through the bridge device where skb_network_offset(skb) is greater than 0 (such as when a bpf program at traffic control egress expands the mac header), the bridge's unconditional pull of the ethernet header leaves skb->data before the network header. In this situation, the direct pointer cast evaluates to a memory address that requires: skb_network_offset(skb) + sizeof(struct ipv6hdr) + sizeof(struct nd_msg) linear bytes, but pskb_may_pull() only validated sizeof(struct ipv6hdr) + sizeof(struct nd_msg) relative to skb->data. ” Patch #3 supersedes the cast, no bug in the assembled series.