[PATCH net] net: team: stop reusing skb after queue override

[PATCH net] net: team: stop reusing skb after queue override

[Weiming Shi]

dev_queue_xmit() consumes the skb regardless of its return value, but
team_queue_override_transmit() treats a non-zero result as if no handoff
occurred.  An AF_PACKET sendto() can therefore make team_xmit() pass a
freed skb to another override port or the mode fallback when the port qdisc
drops the packet.

Report the handoff decision separately from transmit success and stop after
the first override port.  This preserves the existing success and drop
accounting.

  BUG: KASAN: slab-use-after-free in sk_skb_reason_drop
  Read of size 4 at addr ffff88800d5e979c by task poc/131
  Call Trace:
  ...
  sk_skb_reason_drop (net/core/skbuff.c:1220 net/core/skbuff.c:1249)
  team_dummy_transmit (drivers/net/team/team_core.c:502)
  team_xmit (drivers/net/team/team_core.c:1869)
  __dev_direct_xmit (net/core/dev.c:4990)
  packet_xmit (net/packet/af_packet.c:286)
  packet_sendmsg (net/packet/af_packet.c:3140,3172)
  __sys_sendto (net/socket.c:800,815,2281)
  __x64_sys_sendto (net/socket.c:2288,2284)
  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
  Freed by task 131:
  ...
  __tcf_kfree_skb_list (net/sched/sch_generic.c:59)
  __dev_queue_xmit (net/core/dev.c:4353,4884)
  team_xmit (drivers/net/team/team_core.c:832,1867)
  __dev_direct_xmit (net/core/dev.c:4990)
  packet_xmit (net/packet/af_packet.c:286)
  packet_sendmsg (net/packet/af_packet.c:3140,3172)
  __sys_sendto (net/socket.c:800,815,2281)
  __x64_sys_sendto (net/socket.c:2288,2284)
  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
  Kernel panic - not syncing: KASAN: panic_on_warn set ...

Fixes: 8ff5105a2b9d ("team: add support for queue override by setting queue_id for port")
Reported-by: <co+2cf741c3005abc20@bugs.sh>
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/net/team/team_core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index beffbe450612..59dd94271ff1 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -820,7 +820,8 @@ static struct list_head *__team_get_qom_list(struct team *team, u16 queue_id)
 /*
  * note: already called with rcu_read_lock
  */
-static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb)
+static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb,
+					 bool *tx_success)
 {
 	struct list_head *qom_list;
 	struct team_port *port;
@@ -829,8 +830,8 @@ static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb)
 		return false;
 	qom_list = __team_get_qom_list(team, skb->queue_mapping);
 	list_for_each_entry_rcu(port, qom_list, qom_list) {
-		if (!team_dev_queue_xmit(team, port, skb))
-			return true;
+		*tx_success = !team_dev_queue_xmit(team, port, skb);
+		return true;
 	}
 	return false;
 }
@@ -1864,8 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
 	bool tx_success;
 	unsigned int len = skb->len;
 
-	tx_success = team_queue_override_transmit(team, skb);
-	if (!tx_success)
+	if (!team_queue_override_transmit(team, skb, &tx_success))
 		tx_success = READ_ONCE(team->ops.transmit)(team, skb);
 	if (tx_success) {
 		struct team_pcpu_stats *pcpu_stats;
-- 
2.55.0

Re: [PATCH net] net: team: stop reusing skb after queue override

[Hangbin Liu]

Hi Weiming,
On Sat, Sep 26, 2026 at 05:00:39PM +0800, Weiming Shi wrote:
> dev_queue_xmit() consumes the skb regardless of its return value, but
> team_queue_override_transmit() treats a non-zero result as if no handoff
> occurred.  An AF_PACKET sendto() can therefore make team_xmit() pass a
> freed skb to another override port or the mode fallback when the port qdisc
> drops the packet.
> 
> Report the handoff decision separately from transmit success and stop after
> the first override port.  This preserves the existing success and drop
> accounting.
> 
>   BUG: KASAN: slab-use-after-free in sk_skb_reason_drop
>   Read of size 4 at addr ffff88800d5e979c by task poc/131
>   Call Trace:
>   ...
>   sk_skb_reason_drop (net/core/skbuff.c:1220 net/core/skbuff.c:1249)
>   team_dummy_transmit (drivers/net/team/team_core.c:502)
>   team_xmit (drivers/net/team/team_core.c:1869)
>   __dev_direct_xmit (net/core/dev.c:4990)
>   packet_xmit (net/packet/af_packet.c:286)
>   packet_sendmsg (net/packet/af_packet.c:3140,3172)
>   __sys_sendto (net/socket.c:800,815,2281)
>   __x64_sys_sendto (net/socket.c:2288,2284)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>   Freed by task 131:
>   ...
>   __tcf_kfree_skb_list (net/sched/sch_generic.c:59)
>   __dev_queue_xmit (net/core/dev.c:4353,4884)
>   team_xmit (drivers/net/team/team_core.c:832,1867)
>   __dev_direct_xmit (net/core/dev.c:4990)
>   packet_xmit (net/packet/af_packet.c:286)
>   packet_sendmsg (net/packet/af_packet.c:3140,3172)
>   __sys_sendto (net/socket.c:800,815,2281)
>   __x64_sys_sendto (net/socket.c:2288,2284)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>   Kernel panic - not syncing: KASAN: panic_on_warn set ...
> 
> Fixes: 8ff5105a2b9d ("team: add support for queue override by setting queue_id for port")
> Reported-by: <co+2cf741c3005abc20@bugs.sh>
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  drivers/net/team/team_core.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index beffbe450612..59dd94271ff1 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -820,7 +820,8 @@ static struct list_head *__team_get_qom_list(struct team *team, u16 queue_id)
>  /*
>   * note: already called with rcu_read_lock
>   */
> -static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb)
> +static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb,
> +					 bool *tx_success)
>  {
>  	struct list_head *qom_list;
>  	struct team_port *port;
> @@ -829,8 +830,8 @@ static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb)
>  		return false;
>  	qom_list = __team_get_qom_list(team, skb->queue_mapping);
>  	list_for_each_entry_rcu(port, qom_list, qom_list) {
> -		if (!team_dev_queue_xmit(team, port, skb))
> -			return true;
> +		*tx_success = !team_dev_queue_xmit(team, port, skb);
> +		return true;
>  	}


Since dev_queue_xmit() consumes the skb and we should not loop for all
the ports. Is there still a need to using list_for_each_entry_rcu()?

Thanks
Hangbin

>  	return false;
>  }
> @@ -1864,8 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
>  	bool tx_success;
>  	unsigned int len = skb->len;
>  
> -	tx_success = team_queue_override_transmit(team, skb);
> -	if (!tx_success)
> +	if (!team_queue_override_transmit(team, skb, &tx_success))
>  		tx_success = READ_ONCE(team->ops.transmit)(team, skb);
>  	if (tx_success) {
>  		struct team_pcpu_stats *pcpu_stats;
> -- 
> 2.55.0
> 

Re: [PATCH net] net: team: stop reusing skb after queue override

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "Lets fix all the bugs in this area in one series"
---
net: team: stop reusing skb after queue override

This patch fixes a use-after-free in team_xmit(). Before it,
team_queue_override_transmit() treated a non-zero dev_queue_xmit() return
as "no handoff" and then reused the skb, which had already been consumed.
The patch reports the handoff separately from transmit success and only
tries the first override port.

> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index beffbe450612b..59dd94271ff19 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c

[ ... ]

> @@ -1864,8 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
>  	bool tx_success;
>  	unsigned int len = skb->len;
>  
> -	tx_success = team_queue_override_transmit(team, skb);
> -	if (!tx_success)
> +	if (!team_queue_override_transmit(team, skb, &tx_success))
>  		tx_success = READ_ONCE(team->ops.transmit)(team, skb);

[Severity: High]
This is a pre-existing issue, not one introduced by this patch. This fix
relies on the transmit op always consuming the skb. Does broadcast mode
break that rule?

In bc_transmit() in drivers/net/team/team_mode_broadcast.c, the original
skb is only handed off if a txable port was found:

	if (last) {
		ret = !team_dev_queue_xmit(team, last, skb);
		if (!sum_ret)
			sum_ret = ret;
	}
	return sum_ret;

If no port passes team_port_txable(), last is NULL and the skb is neither
sent nor freed. team_xmit() then only increments tx_dropped and returns
NETDEV_TX_OK.

Does this leak the skb and its sk_wmem_alloc charge, since sock_wfree()
never runs?

Can this state be reached? team_adjust_ops() picks the mode transmit op
based only on tx_en_port_count:

	if (!team->tx_en_port_count || !team_is_mode_set(team) ||
	    !team->mode->ops->transmit)
		WRITE_ONCE(team->ops.transmit, team_dummy_transmit);
	else
		WRITE_ONCE(team->ops.transmit, team->mode->ops->transmit);

However, team_port_txable() also requires port->linkup.

For example, say port A has link but its tx is disabled through the
tx_enabled option. Port B is oper-down but tx-enabled, which is the default
after team_port_add(). __team_carrier_check() keeps carrier on because port
A has link, and tx_en_port_count == 1 keeps bc_transmit() installed. Every
packet sent through the team device would then leak.

The same could happen if all tx-enabled ports lose link while transmits
are in flight. It could also happen if user_carrier_enabled keeps carrier
up while all ports are down.

The other transmit ops (roundrobin, random, loadbalance, activebackup and
team_dummy_transmit()) all call dev_kfree_skb_any() when they find no port.

Should bc_transmit() also call dev_kfree_skb_any(skb) when last is NULL?

[ ... ]
-- 
pw-bot: cr

[PATCH net v2 0/2] net: team: fix transmit skb ownership

[Weiming Shi]

The queue override path can pass an skb to a lower device, see that
dev_queue_xmit() returned a drop, and then reuse the consumed skb. Fix
the handoff decision separately from the transmit result.

An AI-generated review of v1 forwarded by Jakub identified a separate
broadcast-mode leak: bc_transmit() returns without consuming the skb
when no port is txable. Fix that in the second patch.

v1: https://lore.kernel.org/r/20260926090039.1714460-1-bestswngs@gmail.com

Changes since v1:
- Use list_first_or_null_rcu() in the queue override helper, as Hangbin
  suggested, since only one override port can receive the skb.
- Add a separate broadcast-mode fix for the no-port skb leak raised in
  the AI-generated review forwarded by Jakub.

Weiming Shi (2):
  net: team: stop reusing skb after queue override
  net: team: free skb when broadcast has no txable port

 drivers/net/team/team_core.c           | 16 ++++++++--------
 drivers/net/team/team_mode_broadcast.c |  2 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.55.0

[PATCH net v2 1/2] net: team: stop reusing skb after queue override

[Weiming Shi]

A nonzero dev_queue_xmit() result does not return ownership of the skb.
When an override port's qdisc drops it, the queue override helper used to
try another port or the mode transmit op with the freed skb.

Return the handoff decision separately from transmit success. Select only
the first override port under RCU and stop after handing it the skb,
regardless of the lower transmit result. Keep the existing success and
drop accounting.

  BUG: KASAN: slab-use-after-free in sk_skb_reason_drop
  Write of size 4 by task poc/5040
  sk_skb_reason_drop (net/core/skbuff.c:1240)
  dev_kfree_skb_any_reason (net/core/dev.c:3470)
  ab_transmit (drivers/net/team/team_mode_activebackup.c:49)
  team_xmit (drivers/net/team/team_core.c:1849)
  __dev_direct_xmit (net/core/dev.c:4934)
  Freed by task 5040:
  __tcf_kfree_skb_list (net/sched/sch_generic.c:43)
  __dev_queue_xmit (net/core/dev.c:4831)
  team_xmit (drivers/net/team/team_core.c:1846)
  Kernel panic - not syncing: KASAN: panic_on_warn set ...

Cc: stable@vger.kernel.org
Fixes: 8ff5105a2b9d ("team: add support for queue override by setting queue_id for port")
Reported-by: <co+2cf741c3005abc20@bugs.sh>
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/net/team/team_core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index feaa75fbf8fc2..3bf7c80f1d8e5 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -820,7 +820,8 @@ static struct list_head *__team_get_qom_list(struct team *team, u16 queue_id)
 /*
  * note: already called with rcu_read_lock
  */
-static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb)
+static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb,
+					 bool *tx_success)
 {
 	struct list_head *qom_list;
 	struct team_port *port;
@@ -828,11 +829,11 @@ static bool team_queue_override_transmit(struct team *team, struct sk_buff *skb)
 	if (!team->queue_override_enabled || !skb->queue_mapping)
 		return false;
 	qom_list = __team_get_qom_list(team, skb->queue_mapping);
-	list_for_each_entry_rcu(port, qom_list, qom_list) {
-		if (!team_dev_queue_xmit(team, port, skb))
-			return true;
-	}
-	return false;
+	port = list_first_or_null_rcu(qom_list, struct team_port, qom_list);
+	if (!port)
+		return false;
+	*tx_success = !team_dev_queue_xmit(team, port, skb);
+	return true;
 }
 
 static void __team_queue_override_port_del(struct team *team,
@@ -1864,8 +1865,7 @@ static netdev_tx_t team_xmit(struct sk_buff *skb, struct net_device *dev)
 	bool tx_success;
 	unsigned int len = skb->len;
 
-	tx_success = team_queue_override_transmit(team, skb);
-	if (!tx_success)
+	if (!team_queue_override_transmit(team, skb, &tx_success))
 		tx_success = READ_ONCE(team->ops.transmit)(team, skb);
 	if (tx_success) {
 		struct team_pcpu_stats *pcpu_stats;
-- 
2.55.0

[PATCH net v2 2/2] net: team: free skb when broadcast has no txable port

[Weiming Shi]

bc_transmit() sends the original skb through the last txable port.
When no port is txable, it returns false without consuming the skb, but
team_xmit() still returns NETDEV_TX_OK. AF_PACKET sends then retain the
skb and its socket write-memory charge.

A TX-enabled, link-down port keeps the broadcast transmit op installed.
Carrier can stay up through a second link-up, TX-disabled port or be
forced on by userspace. Free the original skb on the no-port path while
leaving the existing drop accounting intact.

Cc: stable@vger.kernel.org
Fixes: 5fc889911a99 ("team: add broadcast mode")
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 drivers/net/team/team_mode_broadcast.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/team/team_mode_broadcast.c b/drivers/net/team/team_mode_broadcast.c
index 61d7d79f0c363..69dc25a5847a1 100644
--- a/drivers/net/team/team_mode_broadcast.c
+++ b/drivers/net/team/team_mode_broadcast.c
@@ -37,6 +37,8 @@ static bool bc_transmit(struct team *team, struct sk_buff *skb)
 		ret = !team_dev_queue_xmit(team, last, skb);
 		if (!sum_ret)
 			sum_ret = ret;
+	} else {
+		dev_kfree_skb_any(skb);
 	}
 	return sum_ret;
 }
-- 
2.55.0

Re: [PATCH net v2 0/2] net: team: fix transmit skb ownership

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2 0/2] net: team: fix transmit skb ownership

[Jakub Kicinski]

On Fri,  2 Oct 2026 02:23:24 +0800 Weiming Shi wrote:
> The queue override path can pass an skb to a lower device, see that
> dev_queue_xmit() returned a drop, and then reuse the consumed skb. Fix
> the handoff decision separately from the transmit result.
> 
> An AI-generated review of v1 forwarded by Jakub identified a separate
> broadcast-mode leak: bc_transmit() returns without consuming the skb
> when no port is txable. Fix that in the second patch.

Do not repost just for this but, please, DO NOT post new revisions in
reply to old ones.

See https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
-- 
pv-bot: threaded