> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of Jacob > Keller > Sent: 17 July 2025 22:27 > To: Nguyen, Anthony L <[email protected]>; Intel Wired LAN > <[email protected]>; Loktionov, Aleksandr > <[email protected]> > Cc: Keller, Jacob E <[email protected]>; Grinberg, Vitaly > <[email protected]>; [email protected] > Subject: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix double-call to > ice_deinit_hw() during probe failure > > The following (and similar) KFENCE bugs have recently been found occurring > during certain error flows of the ice_probe() function: > > kernel: ================================================================== > kernel: BUG: KFENCE: use-after-free read in ice_cleanup_fltr_mgmt_struct+0x1d > kernel: Use-after-free read at 0x00000000e72fe5ed (in kfence-#223): > kernel: ice_cleanup_fltr_mgmt_struct+0x1d/0x200 [ice] > kernel: ice_deinit_hw+0x1e/0x60 [ice] > kernel: ice_probe+0x245/0x2e0 [ice] > kernel: > kernel: kfence-#223: <..snip..> > kernel: allocated by task 7553 on cpu 0 at 2243.527621s (198.108303s ago): > kernel: devm_kmalloc+0x57/0x120 > kernel: ice_init_hw+0x491/0x8e0 [ice] > kernel: ice_probe+0x203/0x2e0 [ice] > kernel: > kernel: freed by task 7553 on cpu 0 at 2441.509158s (0.175707s ago): > kernel: ice_deinit_hw+0x1e/0x60 [ice] > kernel: ice_init+0x1ad/0x570 [ice] > kernel: ice_probe+0x22b/0x2e0 [ice] > kernel: > kernel: ================================================================== > > These occur as the result of a double-call to ice_deinit_hw(). This double > call happens if ice_init() fails at any point after calling ice_init_dev(). > > Upon errors, ice_init() calls ice_deinit_dev(), which is supposed to be the > inverse of ice_init_dev(). However, currently ice_init_dev() does not call > ice_init_hw(). Instead, ice_init_hw() is called by ice_probe(). Thus, ice_probe() itself calls ice_deinit_hw() as part of its error cleanup logic. > > This results in two calls to ice_deinit_hw() which results in straight > forward use-after-free violations due to double calling kfree and other > cleanup functions. > > To avoid this double call, move the call to ice_init_hw() into > ice_init_dev(), and remove the now logically unnecessary cleanup from > ice_probe(). This is simpler than the alternative of moving ice_deinit_hw() *out* of ice_deinit_dev(). > > Moving the calls to ice_deinit_hw() requires validating all cleanup paths, > and changing significantly more code. Moving the calls of ice_init_hw() > requires only validating that the new placement is still prior to all HW > structure accesses. > > For ice_probe(), this now delays ice_init_hw() from before ice_adapter_get() to just after it. This is safe, as ice_adapter_get() does not rely on the HW structure. > > For ice_devlink_reinit_up(), the ice_init_hw() is now called after > ice_set_min_max_msix(). This is also safe as that function does not access > the HW structure either. > > This flow makes more logical sense, as ice_init_dev() is mirrored by > ice_deinit_dev(), so it reasonably should be the caller of ice_init_hw(). > It also reduces one extra call to ice_init_hw() since both ice_probe() and ice_devlink_reinit_up() call ice_init_dev(). > > This resolves the double-free and avoids memory corruption and other invalid > memory accesses in the event of a failed probe. > > Fixes: 5b246e533d01 ("ice: split probe into smaller functions") > Signed-off-by: Jacob Keller <[email protected]> > --- > drivers/net/ethernet/intel/ice/devlink/devlink.c | 10 +--------- > drivers/net/ethernet/intel/ice/ice_main.c | 24 +++++++++++------------- > 2 files changed, 12 insertions(+), 22 deletions(-) >
Tested-by: Rinitha S <[email protected]> (A Contingent worker at Intel)
