Re: [PATCH net-next v5 12/13] net: mana: keep the surviving queues when the channel count is reduced

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing `synchronize_net()` on the rollback path of `mana_publish_qset` allows concurrent readers to dereference newly allocated containers after they are freed.

Pre-existing issues:
- [High] u64_stats writer runs with preemption enabled, which can deadlock 32-bit readers in BH context
--

commit ac5cec529175b7a19576c74efe705cb50e0c6d42
Author: Long Li [off-list ref]

net: mana: keep the surviving queues when the channel count is reduced

Split the live set into a kept prefix and a retiring tail. Reductions
allocate only pointer arrays and steering tables, retaining the kept
queues' page pools, buffers, NAPI state and XDP references.

[ ... ]

[Severity: High]
This is a pre-existing issue, but the rollback path here appears to have a
synchronization problem. If mana_publish_qset() fails, it triggers a rollback
that ultimately calls mana_fold_rxq_stats() to update stats:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_fold_rxq_stats() {
    ...
	u64_stats_update_begin(&dst->syncp);
	dst->packets		+= src->packets;
	dst->bytes		+= src->bytes;
	u64_stats_update_end(&dst->syncp);
    ...
}

Since this update runs under the RTNL lock in process context without
disabling preemption or bottom halves, what happens if a softirq interrupts
this writer and executes dev_get_stats() (which calls u64_stats_fetch_begin())?

Could this lead to the reader spinning infinitely waiting for the seqcount
to become even, resulting in a hard lockup on 32-bit architectures?

[ ... ]

[ ... ]

[Severity: High]
If mana_publish_qset() fails, its rollback path restores the old pointers via
mana_qset_install(apc, out_old) but returns err without waiting for an RCU 
grace period (such as via synchronize_net()) to ensure readers that observed
the newq pointers have finished.

Here in mana_set_channels(), mana_discard_split() is called immediately,
which frees the newly allocated containers:

drivers/net/ethernet/microsoft/mana/mana_en.c:mana_discard_split() {
    kfree(newq->tx_qp);
    kfree(newq->rxqs);
    kfree(newq->indir_table);
    kfree(newq->rxobj_table);
    ...
}

Could concurrent RCU-protected readers like ndo_select_queue (which are not
blocked by netif_tx_disable()) observe the new pointers and execute a
use-after-free when these containers are freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909222416.884246-1-longli@microsoft.com?part=12