Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] TOCTOU race and Use-After-Free in mana_get_stats64() and mana_xdp_xmit() due to lockless access of queues. -- --- Patch [1]: [PATCH net v2] net: mana: Return error code from mana_create_rxq() --- commit 61de155c59ba6f86af33fcadf7943152726f385f Author: Aditya Garg <[email protected]> net: mana: Return error code from mana_create_rxq() mana_create_rxq() returns a struct mana_rxq pointer and returns NULL on any failure. The caller, mana_add_rx_queues(), cannot tell what went wrong and hardcodes the error as -ENOMEM. As a result the actual failure reported by the lower layers is masked. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c > b/drivers/net/ethernet/microsoft/mana/mana_en.c > index 9d9bfd116dabc..92bb55935c1c4 100644 > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c [ ... ] > @@ -2964,8 +2964,8 @@ static int mana_add_rx_queues(struct mana_port_context > *apc, > > for (i = 0; i < apc->num_queues; i++) { > rxq = mana_create_rxq(apc, i, &apc->eqs[i], ndev); > - if (!rxq) { > - err = -ENOMEM; > + if (IS_ERR(rxq)) { > + err = PTR_ERR(rxq); > netdev_err(ndev, "Failed to create rxq %d : %d\n", i, > err); > goto out; > } [Severity: High] This is a pre-existing issue, but is there a TOCTOU race and Use-After-Free in mana_get_stats64() and mana_xdp_xmit() due to lockless access of queues? If an administrator changes the MTU or closes the interface, mana_detach() is triggered. It locklessly clears apc->port_is_up, disables TX, and then frees the RX and TX queues using kvfree() directly via mana_dealloc_queues(), without an RCU grace period: mana_detach() apc->port_is_up = false; /* Ensure port state updated before txq state */ smp_wmb(); netif_tx_disable(ndev); if (apc->port_st_save) { err = mana_dealloc_queues(ndev); Concurrently, can unprivileged userspace reading /proc/net/dev (triggering mana_get_stats64()) check apc->port_is_up, evaluate it as true, and then access the freed queues? mana_get_stats64() if (!apc->port_is_up) return; ... for (q = 0; q < num_queues; q++) { rx_stats = &apc->rxqs[q]->stats; Similarly, in mana_xdp_xmit(), the TX lock is temporarily acquired and dropped for each packet in the loop, allowing mana_detach() to complete netif_tx_disable() and proceed with freeing the queue before mana_xdp_xmit() updates the stats: mana_xdp_xmit() for (i = 0; i < n; i++) { if (mana_xdp_xmit_fm(ndev, frames[i], q_idx)) break; count++; } tx_stats = &apc->tx_qp[q_idx]->txq.stats; u64_stats_update_begin(&tx_stats->syncp); Can this lead to a Use-After-Free where memory corruption or a kernel panic occurs? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
