| 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
| ||
|
Message-Id: <20250717-jk-ddp-safe-mode-issue-v1-1-e113b2baed79@intel.com>
Date: Thu, 17 Jul 2025 09:57:08 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Anthony Nguyen <anthony.l.nguyen@...el.com>,
Intel Wired LAN <intel-wired-lan@...ts.osuosl.org>,
Aleksandr Loktionov <aleksandr.loktionov@...el.com>
Cc: Jacob Keller <jacob.e.keller@...el.com>, vgrinber@...hat.com,
netdev@...r.kernel.org
Subject: [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(-)
diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
index 4af60e2f37df..ef49da0590b3 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
@@ -1233,18 +1233,12 @@ static int ice_devlink_reinit_up(struct ice_pf *pf)
struct ice_vsi *vsi = ice_get_main_vsi(pf);
int err;
- err = ice_init_hw(&pf->hw);
- if (err) {
- dev_err(ice_pf_to_dev(pf), "ice_init_hw failed: %d\n", err);
- return err;
- }
-
/* load MSI-X values */
ice_set_min_max_msix(pf);
err = ice_init_dev(pf);
if (err)
- goto unroll_hw_init;
+ return err;
vsi->flags = ICE_VSI_FLAG_INIT;
@@ -1267,8 +1261,6 @@ static int ice_devlink_reinit_up(struct ice_pf *pf)
rtnl_unlock();
err_vsi_cfg:
ice_deinit_dev(pf);
-unroll_hw_init:
- ice_deinit_hw(&pf->hw);
return err;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 0a11b4281092..c44bb8153ad0 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4739,6 +4739,12 @@ int ice_init_dev(struct ice_pf *pf)
struct ice_hw *hw = &pf->hw;
int err;
+ err = ice_init_hw(hw);
+ if (err) {
+ dev_err(dev, "ice_init_hw failed: %d\n", err);
+ return err;
+ }
+
ice_init_feature_support(pf);
err = ice_init_ddp_config(hw, pf);
@@ -4759,7 +4765,7 @@ int ice_init_dev(struct ice_pf *pf)
err = ice_init_pf(pf);
if (err) {
dev_err(dev, "ice_init_pf failed: %d\n", err);
- return err;
+ goto unroll_hw_init;
}
pf->hw.udp_tunnel_nic.set_port = ice_udp_tunnel_set_port;
@@ -4803,6 +4809,8 @@ int ice_init_dev(struct ice_pf *pf)
ice_clear_interrupt_scheme(pf);
unroll_pf_init:
ice_deinit_pf(pf);
+unroll_hw_init:
+ ice_deinit_hw(hw);
return err;
}
@@ -5330,17 +5338,9 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
if (ice_is_recovery_mode(hw))
return ice_probe_recovery_mode(pf);
- err = ice_init_hw(hw);
- if (err) {
- dev_err(dev, "ice_init_hw failed: %d\n", err);
- return err;
- }
-
adapter = ice_adapter_get(pdev);
- if (IS_ERR(adapter)) {
- err = PTR_ERR(adapter);
- goto unroll_hw_init;
- }
+ if (IS_ERR(adapter))
+ return PTR_ERR(adapter);
pf->adapter = adapter;
err = ice_init(pf);
@@ -5366,8 +5366,6 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
ice_deinit(pf);
unroll_adapter:
ice_adapter_put(pdev);
-unroll_hw_init:
- ice_deinit_hw(hw);
return err;
}
--
2.50.0.349.ga842a77808e9
Powered by blists - more mailing lists