https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=6 [...] > > @@ -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 What are the two callers? I only find batadv_mcast_forw_mode_by_count() > 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? Will not modify this patch for this request. Linus (or anyone else) can deal with it: https://www.open-mesh.org/issues/466 > > Related to that: batadv_interface_tx() does not re-acquire ethhdr after > batadv_mcast_forw_mode() returns, even though the new comment warns about > reallocation. The only later use, is_broadcast_ether_addr(ethhdr->h_dest), > is evaluated before the call, The later use is before? This is crazy > so nothing is dereferenced stale right now. > Is it worth re-acquiring it anyway, or noting in the comment that ethhdr > must not be used past this point? Will not modify the patch for this. > > > switch (forw_mode) { > > [ ... ] > > > @@ -355,8 +360,6 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb, > > } > > } > > > > - batadv_skb_set_priority(skb, 0); > > - > > /* ethernet packet should be broadcasted */ > > if (do_bcast) { > > primary_if = batadv_primary_if_get_selected(bat_priv); > > Discovered by: opus-5 · Missed by: gpt-5-6-sol, sashiko-gemini · Confirmed by: gpt-5-6-sol > This is a pre-existing issue and not caused by this patch, but while the > priority handling is being looked at: should the multicast forwarding path > also set skb->priority? This is a feature (or maybe minor bugfix) request. Will definitely not deal with it here. Documented it at https://www.open-mesh.org/issues/467 Regards, Sven