Project

General

Profile

Actions

Feature #466

open

mcast: document functions which might reallocate skb

Added by Sven Eckelmann about 2 months ago. Updated about 2 months ago.

Status:
New
Priority:
Normal
Target version:
-
Start date:
08/22/2026
Due date:
% Done:

0%

Estimated time:

Description

See https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=6

diff --git a/net/batman-adv/mesh-interface.c b/net/batman-adv/mesh-interface.c
index 8e55b61dd2a64..e202088cf1fd4 100644
--- a/net/batman-adv/mesh-interface.c
+++ b/net/batman-adv/mesh-interface.c
@ -305,6 +305,8 @ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
if (batadv_compare_eth(ethhdr->h_dest, ectp_addr))
goto dropped;

+ batadv_skb_set_priority(skb, 0);
+
gw_mode = READ_ONCE(bat_priv->gw.mode);
if (is_multicast_ether_addr(ethhdr->h_dest)) {
/* if gw mode is off, broadcast every packet */

[ ... ]

> > @@ -338,6 +340,9 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
> >  
> >  send:
> >          if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
> > +            /* WARNING batadv_mcast_forw_mode might add more headers
> > +             * in front of the skb. and might even reallocate the skb
> > +             */
> >              forw_mode = batadv_mcast_forw_mode(bat_priv, skb, vid,
> >                                 &mcast_is_routable);
> 

Discovered by: opus-5 · Missed by: gpt-5-6-sol, sashiko-gemini · Confirmed by: gpt-5-6-sol
This isn't a bug introduced by this patch, but would it make sense to put
this warning into the kernel-doc of the callee instead of (or in addition
to) the call site?

Today only batadv_mcast_forw_push() carries the contract in
net/batman-adv/multicast_forw.c:

>   * Warning: This function may reallocate the skb data buffer via
>   * batadv_mcast_forw_expand_head()/... Any pointer into the skb data (e.g.
>   * obtained from skb->data or eth_hdr()) before this call must be
>   * considered invalid afterwards and has to be reacquired.
> 

The two callers
in net/batman-adv/multicast.c do not mention it.
batadv_mcast_forw_mode_by_count() only says:
>   * For a multicast packet with multiple destination originators, checks which
>   * mode to use. For BATADV_FORW_MCAST it also encapsulates the packet with a
>   * complete batman-adv multicast header.
> 

and the exported entry point batadv_mcast_forw_mode() (declared in
net/batman-adv/multicast.h) documents only:
>   * Return: The forwarding mode as enum batadv_forw_mode.
> 

Since batadv_mcast_forw_mode() is the exported symbol, wouldn't a future
caller reading its kernel-doc have no way to know that skb->data was pushed
or that the buffer may have been reallocated?

Actions #1

Updated by Sven Eckelmann about 2 months ago

  • Tracker changed from Bug to Feature
Actions

Also available in: Atom PDF