Re: [PATCH net v2] net: neigh: Reallocate headroom if necessary in neigh_hh_bridge()

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]

> On Mon, May 11, 2026 at 05:57:39PM +0200, Lorenzo Bianconi wrote:
> > diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> > index 2dfee6d4258a..c2b6196705ef 100644
> > --- a/include/net/neighbour.h
> > +++ b/include/net/neighbour.h
> > @@ -487,16 +487,24 @@ static inline int neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
> >  }
> >  
> >  #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
> > -static inline int neigh_hh_bridge(struct hh_cache *hh, struct sk_buff *skb)
> > +static inline struct sk_buff *
> > +neigh_hh_bridge(struct hh_cache *hh, struct sk_buff *skb)
> >  {
> > -	unsigned int seq, hh_alen;
> > +	unsigned int seq, hh_alen = HH_DATA_ALIGN(ETH_HLEN);
> > +
> > +	if (unlikely(skb_headroom(skb) < hh_alen ||
> > +		     skb_header_cloned(skb) || skb_shared(skb))) {
> > +		skb = skb_expand_head(skb, hh_alen);
> 
> I don't think this is correct... The comment above skb_expand_head()
> says that it will generate a warning if there is sufficient headroom in
> the packet.
> 
> I assumed that you would just call skb_cow_head() like the AI review
> suggested. There's skb_share_check() in br_handle_frame(), so no need to
> worry about the skb being shared.

Right, I was reviewing it. I will fix it in v3.

Regards,
Lorenzo

> 
> > +		if (!skb)
> > +			return NULL;
> > +	}
> >  
> >  	do {
> >  		seq = read_seqbegin(&hh->hh_lock);
> > -		hh_alen = HH_DATA_ALIGN(ETH_HLEN);
> >  		memcpy(skb->data - hh_alen, hh->hh_data, ETH_ALEN + hh_alen - ETH_HLEN);
> >  	} while (read_seqretry(&hh->hh_lock, seq));
> > -	return 0;
> > +
> > +	return skb;
> >  }
> >  #endif
> >  
> > diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> > index 0ab1c94db4b9..8d21c88c2116 100644
> > --- a/net/bridge/br_netfilter_hooks.c
> > +++ b/net/bridge/br_netfilter_hooks.c
> > @@ -297,7 +297,12 @@ int br_nf_pre_routing_finish_bridge(struct net *net, struct sock *sk, struct sk_
> >  				goto free_skb;
> >  			}
> >  
> > -			neigh_hh_bridge(&neigh->hh, skb);
> > +			skb = neigh_hh_bridge(&neigh->hh, skb);
> > +			if (!skb) {
> > +				neigh_release(neigh);
> > +				return -ENOMEM;
> > +			}
> 
> This part looks correct.
> 
> > +
> >  			skb->dev = br_indev;
> >  
> >  			ret = br_handle_frame_finish(net, sk, skb);
> > 
> > ---
> > base-commit: a450063ef86b9967234ca1f896c0d77400c74f11
> > change-id: 20260508-nf-neigh_hh_bridge-fix-9ab775ee23c6
> > 
> > Best regards,
> > -- 
> > Lorenzo Bianconi <lorenzo@kernel.org>
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]