> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > > Pre-existing issues: > - [High] mana_xdp_set() is vulnerable to a race condition with RDMA vport > creation because it continues to use the legacy mana_detach()/mana_attach() > sequence without blocking RDMA, leaving the interface permanently down if > a race occurs. > -- > > --- Patch [5]: [PATCH net-next v2 05/13] net: mana: swap queue sets in > mana_change_mtu --- commit > 07ab252b810cb193b26b16516fc0a40653d2b458 > Author: Long Li <[email protected]> > > net: mana: swap queue sets in mana_change_mtu > > The RX buffer layout depends on the MTU, so changing it rebuilds the > queues. Convert mana_change_mtu() to pre-allocate and swap. > > The MTU becomes part of the queue-set configuration, so a new set can be > built for the new MTU while the running one still serves traffic at the old > one, and ndev->mtu is updated only once the new set is live. > Previously it was written before mana_attach() and rolled back on failure, so > a failed change was briefly visible to the stack. > > > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c > > b/drivers/net/ethernet/microsoft/mana/mana_en.c > > index c0f31b386536a..c858a58577dfb 100644 > > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c > > @@ -884,35 +884,49 @@ int mana_pre_alloc_rxbufs(struct > mana_port_context *mpc, int new_mtu, int num_qu > > return -ENOMEM; > > } > > > > +/* ndev->mtu is updated only once the new set is live > > +(mana_publish_qset), so > > + * a failed allocation leaves the queues and the advertised MTU > untouched. > > + */ > > static int mana_change_mtu(struct net_device *ndev, int new_mtu) { > > struct mana_port_context *mpc = netdev_priv(ndev); > > - unsigned int old_mtu = ndev->mtu; > > + struct mana_port_context *scratch; > > + struct mana_qset newq, oldq; > > int err; > > > > - /* Pre-allocate buffers to prevent failure in mana_attach later */ > > - err = mana_pre_alloc_rxbufs(mpc, new_mtu, mpc->num_queues); > > - if (err) { > > - netdev_err(ndev, "Insufficient memory for new MTU\n"); > > - return err; > > + /* Port is down: no queues to rebuild, just record the new MTU. > > + * mana_open() will size the RX buffers accordingly. > > + */ > > + if (!mpc->port_is_up) { > > + mpc->configured_mtu = new_mtu; > > + WRITE_ONCE(ndev->mtu, new_mtu); > > + return 0; > > } > > > > - err = mana_detach(ndev, false); > > - if (err) { > > - netdev_err(ndev, "mana_detach failed: %d\n", err); > > - goto out; > > - } > > [Severity: High] > This isn't a bug introduced by this patch, but since it converts > mana_change_mtu() to avoid the mana_detach()/mana_attach() sequence, is > mana_xdp_set() still vulnerable to a race condition with RDMA vport > creation? > > Looking at mana_xdp_set() in > drivers/net/ethernet/microsoft/mana/mana_bpf.c, > it still uses the legacy detach/attach pattern without blocking RDMA: > > err = mana_detach(ndev, false); > if (err) { > netdev_err(ndev, > "mana_detach failed at xdp set: %d\n", err); > NL_SET_ERR_MSG_MOD(extack, > "XDP: Re-config failed at detach"); > goto err_dealloc_rxbuffs; > } > > err = mana_attach(ndev); > > Because mana_xdp_set() drops vport_use_count to 0 without holding > vport_mutex or setting channel_changing = true, couldn't an RDMA > application successfully acquire the vport in the middle of XDP > reconfiguration? > > If a race occurs, mana_attach() would fail with -EBUSY because RDMA owns > the vport, leaving the Ethernet port permanently down and requiring manual > intervention to recover.
It's fixed by 06/13 patch. Long
