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

[Hangbin Liu <hangbin.liu@linux.dev>]

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
>