Re: [PATCH 1/2] ice: restore DDP state during PFR recovery
From: Simon Horman <horms@kernel.org>
Date: 2026-09-05 16:44:07
Also in:
intel-wired-lan, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/#/patchset/20260903074706.602087-2-aaron.ma@canonical.com?part=1 --- Subject: ice: restore DDP state during PFR recovery This patch serializes rebuilds across PFs of the same adapter while allowing each PFR to complete independently. It reloads the DDP package from its cached copy and restores the default DVM recipes before rebuilding the remaining PF state. Fixes: 462acf6aca85 ("ice: Enable DDP package download") [ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index d88835482d3aa..a97a1941cec6a 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c@@ -659,7 +659,9 @@ static void ice_do_reset(struct ice_pf *pf, enum ice_reset_req reset_type) */ if (reset_type == ICE_RESET_PFR) { pf->pfr_count++; + mutex_lock(&pf->adapter->rebuild_lock); ice_rebuild(pf, reset_type); + mutex_unlock(&pf->adapter->rebuild_lock);
[Severity: High]
Can this cause a use-after-free of pf->adapter during concurrent device
removal?
If a device reset triggered in the service task takes longer than the
timeout, ice_remove() proceeds to teardown and calls ice_adapter_put().
For the last PF on the adapter, this drops the refcount to zero and frees
the adapter structure memory.
However, the service task is only explicitly halted by ice_deinit_dev()
after this point:
ice_remove()
...
ice_adapter_put(pdev);
ice_deinit_hw(&pf->hw);
ice_deinit_dev(pf); /* service task halted here */
If the service task was executing ice_do_reset(), it will eventually
complete the rebuild and call mutex_unlock(&pf->adapter->rebuild_lock),
dereferencing the freed adapter structure.
quoted hunk ↗ jump to hunk
clear_bit(ICE_PREPARED_FOR_RESET, pf->state); clear_bit(ICE_PFR_REQ, pf->state); wake_up(&pf->reset_wait_queue);@@ -704,7 +706,9 @@ static void ice_reset_subtask(struct ice_pf *pf) } else { /* done with reset. start rebuild */ pf->hw.reset_ongoing = false; + mutex_lock(&pf->adapter->rebuild_lock); ice_rebuild(pf, reset_type); + mutex_unlock(&pf->adapter->rebuild_lock);
[Severity: High] Similarly, does this reset subtask release the rebuild_lock on a potentially freed adapter structure if the remove timeout expired while the rebuild was in progress?
/* clear bit to resume normal operations, but * ICE_NEEDS_RESTART bit is set in case rebuild failed */
[ ... ]