Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] If `mana_smc_teardown_hwc()` fails, the driver proceeds to unconditionally destroy the hardware queues and free their backing DMA memory, which can lead to a DMA use-after-free by the device. -- commit 0af150088ed8994a134d0a328a359e4cea4bbce5 Author: Long Li <[email protected]> net: mana: track when the HWC has been handed to the PF This commit introduces a new setup_active flag to reliably track whether the hardware queues have been handed to the physical function, addressing a gap where the previous queue depth check was insufficient. This ensures accurate state tracking during teardown to prepare for a future reinitialization path. > diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c > b/drivers/net/ethernet/microsoft/mana/hw_channel.c > index 263e7c4e29341..75fdccdc8c482 100644 > --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c > +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c [ ... ] > @@ -815,13 +815,20 @@ void mana_hwc_destroy_channel(struct gdma_context *gc) > if (!hwc) > return; > > - /* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's > - * non-zero, the HWC worked and we should tear down the HWC here. > + /* Tear down only if setup_hwc() handed the queues to the PF. Until > + * then the device never saw them, so there is nothing to undo. > */ > - if (gc->max_num_cqs > 0) { > - mana_smc_teardown_hwc(&gc->shm_channel, false); > - gc->max_num_cqs = 0; > + if (hwc->setup_active) { > + /* Only a successful teardown invalidates the MST entries. If > + * it fails the device may still be using the queues, so leave > + * the flag set rather than record a clean teardown. > + */ > + if (!mana_smc_teardown_hwc(&gc->shm_channel, false)) > + hwc->setup_active = false; > + else > + dev_err(hwc->dev, "Failed to tear down HWC\n"); > } > + gc->max_num_cqs = 0; > > if (hwc->txq) > mana_hwc_destroy_wq(hwc, hwc->txq); [Severity: High] This is a pre-existing issue, but if mana_smc_teardown_hwc() fails, does the driver proceed to unconditionally destroy the hardware queues and free their backing DMA memory? As the newly added comment acknowledges that "the device may still be using the queues", calling mana_hwc_destroy_wq() unconditionally unmaps and frees the DMA memory. If the physical function hardware is still active, could this result in a DMA use-after-free leading to memory corruption or IOMMU faults? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
