Re: [PATCH net 8/9] batman-adv: mcast: linearize skbuff for packet generation
Sven Eckelmann <[email protected]>
| Newsgroups | org.open-mesh.lists.batman,org.kernel.vger.netdev,org.kernel.vger.stable |
|---|---|
| Message-ID | <3908873.MHq7AAxBmi@sven-desktop> |
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=8 [...] > Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini > This isn't a bug introduced by this patch, but the new comment states a > whole-skb requirement that the existing kernel-doc contradicts. > > batadv_mcast_forw_scrape() documents only a prefix requirement: > > * Caller needs to ensure that the region from the skb data's start up > * to/including the to be removed bytes are linearized. > > while its body asserts the entire skb is linear: > > SKB_LINEAR_ASSERT(skb); > > with, in include/linux/skbuff.h: > > #define SKB_LINEAR_ASSERT(skb) BUG_ON(skb_is_nonlinear(skb)) > > Similarly, batadv_mcast_forw_packet()'s kernel-doc lists the caller > obligations (set network/transport header, free the skb) but does not > mention linearity at all; only the in-body comment mentions it, and it > also understates the assert: > > /* (at least) TVLV part needs to be linearized */ > SKB_LINEAR_ASSERT(skb); > > Should the kernel-doc of batadv_mcast_forw_scrape() and > batadv_mcast_forw_packet() be updated to say the whole skb must be linear? > > Along the same lines, the invariant this patch establishes is not recorded > at either interface boundary: batadv_mcast_forw_push()'s kernel-doc does > not mention that on success the skb is linear, and neither the > batadv_mcast_forw_mcsend() kernel-doc nor its declaration in > net/batman-adv/multicast.h mentions the precondition that > batadv_mcast_forw_packet() enforces with BUG_ON(). > > No in-tree caller violates this today: batadv_recv_mcast_packet() unshares > and linearizes, and the batadv_mcast_forw_mcsend() path is now covered by > the skb_linearize() added here. Would documenting the requirement help > avoid a future caller reintroducing the BUG()? Will definitely not deal with this documentation work this bugfix patch. Linus or anyone else interested can work on it: https://www.open-mesh.org/issues/468 Regards, Sven
signature.asc
(application/pgp-signature, 228 B)
-----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQS81G/PswftH/OW8cVND3cr0xT1ywUCaomK7gAKCRBND3cr0xT1 y0dHAQDkkm6Ghk24ASYO7aRNwmGivKGlfgreBB+LR5M7otgeqQEAq/Vslc4Hbk7p /GrkDJ3rDWy41cTXnSztK3BMHLH8xAQ= =lW4r -----END PGP SIGNATURE-----