Re: [PATCH net] net: dsa: Fix skb ownership in taggers
From: David Yang <mmyangfl@gmail.com>
Date: 2026-06-16 06:55:14
On Tue, Jun 16, 2026 at 6:33 AM Linus Walleij [off-list ref] wrote:
The tag_8021q.c tagger calls vlan_insert_tag() in dsa_8021q_xmit(). vlan_insert_tag() will consume the skb with kfree_skb() on failure and return NULL. When NULL is returned as error code to ->xmit() in dsa_user_xmit() it will free the same skb again leading to a double-free. The idea of dsa_user_xmit() and dsa_switch_rcv() dropping the skb they held before the call to ->xmit() and ->rcv() is conceptually wrong: the pattern elsewhere in the networking code is that consumers drop their skb:s on failure. Modify the ->xmit() and ->rcv() call sites to not drop the SKB if the taggers return NULL from any of these calls. Move those drops into the taggers so every callback error path that retains ownership consumes the skb before returning NULL. Keep the existing helper ownership rules: VLAN insertion helpers already free on failure (this is the case in tag_8021q.c), while deferred transmit paths either transfer the skb reference to worker context or hold a worker reference with skb_get() and drop the caller's reference. For SJA1105 meta RX, transfer the buffered stampable skb under the meta lock and return NULL while the skb is waiting for its meta frame: the skb is not dropped in this case. Reported-by: Sashiko AI Review <sashiko-bot@kernel.org> Closes: https://lore.kernel.org/r/20260610153952.1685895-1-kuba@kernel.org/ (local) Suggested-by: Jakub Kicinski <kuba@kernel.org> Assisted-by: Codex:gpt-5-5 Signed-off-by: Linus Walleij <linusw@kernel.org>
Better to use goto err, but I'm fine with the current patch. In either case, Acked-by: David Yang <mmyangfl@gmail.com> # yt921x
--- net/dsa/tag_yt921x.c | 7 ++++++-