netdev - RE: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix double-call to ice_deinit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <IA1PR11MB6241800D4B6DA0F4BC99DF1D8B31A@IA1PR11MB6241.namprd11.prod.outlook.com>
Date: Mon, 18 Aug 2025 16:21:45 +0000
From: "Rinitha, SX" <sx.rinitha@...el.com>
To: "Keller, Jacob E" <jacob.e.keller@...el.com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, Intel Wired LAN
	<intel-wired-lan@...ts.osuosl.org>, "Loktionov, Aleksandr"
	<aleksandr.loktionov@...el.com>
CC: "Keller, Jacob E" <jacob.e.keller@...el.com>, "Grinberg, Vitaly"
	<vgrinber@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix double-call to
 ice_deinit_hw() during probe failure

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of Jacob Keller
> Sent: 17 July 2025 22:27
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>; Loktionov, Aleksandr <aleksandr.loktionov@...el.com>
> Cc: Keller, Jacob E <jacob.e.keller@...el.com>; Grinberg, Vitaly <vgrinber@...hat.com>; netdev@...r.kernel.org
> 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 <jacob.e.keller@...el.com>
> ---
> 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 <sx.rinitha@...el.com> (A Contingent worker at Intel)