> 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? >
This is a pre-existing issue. The patch set implements multi queue depth support and doesn't attempt to fix the existing issue. If needed, I will submit a separate patch to fix the existing issue. The patch is not related to this patch series. Long
