Netdev List
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Weiming Shi <bestswngs@gmail.com>
Cc: Jiri Pirko <jiri@resnulli.us>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, Xiang Mei <xmei5@asu.edu>
Subject: Re: [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change
Date: Tue, 19 May 2026 16:21:45 -0700	[thread overview]
Message-ID: <20260519162145.524da69e@kernel.org> (raw)
In-Reply-To: <agwJNX-q0NmmrMpy@Air.local>

On Tue, 19 May 2026 16:51:28 +0800 Weiming Shi wrote:
> On 26-05-18 14:22, Jakub Kicinski wrote:
> > On Mon, 18 May 2026 17:51:54 +0800 Weiming Shi wrote:  
> > > On 26-05-10 09:59, Jakub Kicinski wrote:  
> > > Apologies for the late reply and for rushing v3.
> > > 
> > > I was muddling two things. On teardown synchronize_net() is the protection,
> > > the release/acquire is for the setup path where init() writes
> > > mode_priv before team_adjust_ops() publishes the handler.
> > > 
> > > If that makes sense I'll send v4 with the corrected commit message.  
> > 
> > Can you provide more details for the init() path race?
> > What's the sequence of events?  
> 
> With loadbalance mode:
>  
>   lb_init() stores select_tx_port_func (team_mode_loadbalance.c:595).
>   When a port is later enabled, team_adjust_ops() publishes
>   lb_transmit with a plain store (team_core.c:539).
> 
> Without the release/acquire, a concurrent team_xmit() on a weakly-ordered 
> arch can see lb_transmit but not the select_tx_port_func store, and lb_transmit 
> dereferences it at line 227.
> 
> I'll send a PoC in the next mail so you can reproduce it.

Not sure this is enough. But feel free to send the v3 if you prefer.

From looking at the repro it seems like you never add any ports?
I suspect that the author of this code assumed that if there are 
no ports there must be no traffic, so it's safe to be flipping the
modes. I'd rather prevent the race than make it safe. Could we defer
setting the real handler until after the first port is added?

  reply	other threads:[~2026-05-19 23:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-09 18:18 [PATCH net v2] net: team: fix NULL pointer dereference in team_xmit during mode change Weiming Shi
2026-05-10 15:25 ` Jakub Kicinski
2026-05-10 16:06   ` Weiming Shi
2026-05-10 16:59     ` Jakub Kicinski
2026-05-18  9:51       ` Weiming Shi
2026-05-18 21:22         ` Jakub Kicinski
2026-05-19  8:51           ` Weiming Shi
2026-05-19 23:21             ` Jakub Kicinski [this message]
2026-05-19  8:57           ` Weiming Shi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260519162145.524da69e@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=bestswngs@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=xmei5@asu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox