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.

> +             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 <[email protected]>
> 

Reply via email to