Re: [Intel-wired-lan] [PATCH iwl-net v1] ice: fix devlink reload call trace

2026-01-15 Thread Rinitha, SX
> -Original Message-
> From: Intel-wired-lan  On Behalf Of Paul 
> Greenwalt
> Sent: 29 December 2025 14:23
> To: [email protected]
> Cc: Greenwalt, Paul ; Loktionov, Aleksandr 
> 
> Subject: [Intel-wired-lan] [PATCH iwl-net v1] ice: fix devlink reload call 
> trace
>
> Commit 4da71a77fc3b ("ice: read internal temperature sensor") introduced 
> internal temperature sensor reading via HWMON. ice_hwmon_init() was added to 
> ice_init_feature() and ice_hwmon_exit() was added to ice_remove(). As a 
> result if devlink reload is used to reinit the device and then the driver is 
> removed, a call trace can occur.
> 
> BUG: unable to handle page fault for address: c0fd4b5d Call Trace:
> string+0x48/0xe0
> vsnprintf+0x1f9/0x650
> sprintf+0x62/0x80
> name_show+0x1f/0x30
> dev_attr_show+0x19/0x60
>
> The call trace repeats approximately every 10 minutes when system monitoring 
> tools (e.g., sadc) attempt to read the orphaned hwmon sysfs attributes that 
> reference freed module memory.
>
> The sequence is:
> 1. Driver load, ice_hwmon_init() gets called from ice_init_feature() 2. 
> Devlink reload down, flow does not call ice_remove() 3. Devlink reload up, 
> ice_hwmon_init() gets called from
   ice_init_feature() resulting in a second instance 4. Driver unload, 
ice_hwmon_exit() called from ice_remove() leaving the
   first hwmon instance orphaned with dangling pointer
>
> Fix this by moving ice_hwmon_exit() from ice_remove() to
> ice_deinit_features() to ensure proper cleanup symmetry with ice_hwmon_init().
>
> Fixes: 4da71a77fc3b ("ice: read internal temperature sensor")
> Reviewed-by: Aleksandr Loktionov 
> Signed-off-by: Paul Greenwalt 
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>

Tested-by: Rinitha S  (A Contingent worker at Intel)


Re: [Intel-wired-lan] [PATCH iwl-net v1] ice: fix devlink reload call trace

2025-12-29 Thread Paul Menzel

Dear Paul,


Thank you for your patch.

Am 29.12.25 um 13:52 schrieb Paul Greenwalt:

Commit 4da71a77fc3b ("ice: read internal temperature sensor") introduced
internal temperature sensor reading via HWMON. ice_hwmon_init() was added
to ice_init_feature() and ice_hwmon_exit() was added to ice_remove(). As a
result if devlink reload is used to reinit the device and then the driver
is removed, a call trace can occur.

BUG: unable to handle page fault for address: c0fd4b5d
Call Trace:
  string+0x48/0xe0
  vsnprintf+0x1f9/0x650
  sprintf+0x62/0x80
  name_show+0x1f/0x30
  dev_attr_show+0x19/0x60

The call trace repeats approximately every 10 minutes when system
monitoring tools (e.g., sadc) attempt to read the orphaned hwmon sysfs
attributes that reference freed module memory.

The sequence is:
1. Driver load, ice_hwmon_init() gets called from ice_init_feature()
2. Devlink reload down, flow does not call ice_remove()
3. Devlink reload up, ice_hwmon_init() gets called from
ice_init_feature() resulting in a second instance
4. Driver unload, ice_hwmon_exit() called from ice_remove() leaving the
first hwmon instance orphaned with dangling pointer

Fix this by moving ice_hwmon_exit() from ice_remove() to
ice_deinit_features() to ensure proper cleanup symmetry with
ice_hwmon_init().


Great commit message. For the summary/title/subject I’d make it more 
specific. Maybe:


ice: Move ice_hwmon_exit() to ice_deinit_features() to avoid accessing 
freed memory


(Is that UAF, use after free?)


Fixes: 4da71a77fc3b ("ice: read internal temperature sensor")
Reviewed-by: Aleksandr Loktionov 
Signed-off-by: Paul Greenwalt 
---
  drivers/net/ethernet/intel/ice/ice_main.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c 
b/drivers/net/ethernet/intel/ice/ice_main.c
index 71b85bf7b033..c7991fb80a86 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4836,6 +4836,7 @@ static void ice_deinit_features(struct ice_pf *pf)
ice_dpll_deinit(pf);
if (pf->eswitch_mode == DEVLINK_ESWITCH_MODE_SWITCHDEV)
xa_destroy(&pf->eswitch.reprs);
+   ice_hwmon_exit(pf);
  }
  
  static void ice_init_wakeup(struct ice_pf *pf)

@@ -5439,8 +5440,6 @@ static void ice_remove(struct pci_dev *pdev)
ice_free_vfs(pf);
}
  
-	ice_hwmon_exit(pf);

-
if (!ice_is_safe_mode(pf))
ice_remove_arfs(pf);
  


Reviewed-by: Paul Menzel 


Kind regards,

Paul