[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