[PATCH net v3] net: e1000: fix warning in iounmap on probe failure

[PATCH net v3] net: e1000: fix warning in iounmap on probe failure

[Svyatoslav Nikolenko]

When e1000_probe() fails, the shared error handling ladder attempts to
iounmap() the hw->ce4100_gbe_mdio_base_virt pointer. If the hardware is
not a CE4100, this pointer remains uninitialized (NULL). On architectures
like x86, passing a NULL pointer to iounmap() triggers a WARN_ON_ONCE,
which is fatal under panic_on_warn.

Fix this by conditionally unmapping the CE4100 MDIO base only if the
mac_type is e1000_ce4100, matching the exact logic used in e1000_remove().
The hw->hw_addr iounmap() remains unconditional since it is guaranteed to
be valid for all error paths reaching the err_sw_init label.

Reported-by: syzbot+ca1ef9e2e234b8d3599b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ca1ef9e2e234b8d3599b
Fixes: 13acde8fffc0af ("e1000: Fix the CE4100 bus type for the MDIO/PHY registers")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Svyatoslav Nikolenko <nsvatoslav515@gmail.com>
---
v3:
  - Switched from pointer null-checks to checking hw->mac_type == e1000_ce4100
  - Removed redundant check for hw->hw_addr since it cannot be NULL here
  - Added missing Cc: stable tag and Closes tag
v2: 
  - Expanded commit message to answer reviewer questions (reproduction details)
  - Added appropriate Fixes tags
  - Added Reviewed-by tag from Aleksandr Loktionov

 drivers/net/ethernet/intel/e1000/e1000_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index d7f5c6f16142..156903089fa6 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1227,8 +1227,10 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	kfree(adapter->rx_ring);
 err_dma:
 err_sw_init:
-	iounmap(hw->ce4100_gbe_mdio_base_virt);
-	iounmap(hw->hw_addr);
+	if (hw->ce4100_gbe_mdio_base_virt)
+		iounmap(hw->ce4100_gbe_mdio_base_virt);
+	if (hw->hw_addr)
+		iounmap(hw->hw_addr);
 err_ioremap:
 	disable_dev = !test_and_set_bit(__E1000_DISABLED, &adapter->flags);
 	free_netdev(netdev);
-- 
2.47.3

Re: [PATCH net v3] net: e1000: fix warning in iounmap on probe failure

[Святослав Ніколенко]

Please drop this version. I accidentally sent this without staging my
local changes.
Sending v4 shortly with the actual code.


чт, 24 вер. 2026 р. о 12:52 Svyatoslav Nikolenko <nsvatoslav515@gmail.com> пише:
>
> When e1000_probe() fails, the shared error handling ladder attempts to
> iounmap() the hw->ce4100_gbe_mdio_base_virt pointer. If the hardware is
> not a CE4100, this pointer remains uninitialized (NULL). On architectures
> like x86, passing a NULL pointer to iounmap() triggers a WARN_ON_ONCE,
> which is fatal under panic_on_warn.
>
> Fix this by conditionally unmapping the CE4100 MDIO base only if the
> mac_type is e1000_ce4100, matching the exact logic used in e1000_remove().
> The hw->hw_addr iounmap() remains unconditional since it is guaranteed to
> be valid for all error paths reaching the err_sw_init label.
>
> Reported-by: syzbot+ca1ef9e2e234b8d3599b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ca1ef9e2e234b8d3599b
> Fixes: 13acde8fffc0af ("e1000: Fix the CE4100 bus type for the MDIO/PHY registers")
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Svyatoslav Nikolenko <nsvatoslav515@gmail.com>
> ---
> v3:
>   - Switched from pointer null-checks to checking hw->mac_type == e1000_ce4100
>   - Removed redundant check for hw->hw_addr since it cannot be NULL here
>   - Added missing Cc: stable tag and Closes tag
> v2:
>   - Expanded commit message to answer reviewer questions (reproduction details)
>   - Added appropriate Fixes tags
>   - Added Reviewed-by tag from Aleksandr Loktionov
>
>  drivers/net/ethernet/intel/e1000/e1000_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index d7f5c6f16142..156903089fa6 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -1227,8 +1227,10 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         kfree(adapter->rx_ring);
>  err_dma:
>  err_sw_init:
> -       iounmap(hw->ce4100_gbe_mdio_base_virt);
> -       iounmap(hw->hw_addr);
> +       if (hw->ce4100_gbe_mdio_base_virt)
> +               iounmap(hw->ce4100_gbe_mdio_base_virt);
> +       if (hw->hw_addr)
> +               iounmap(hw->hw_addr);
>  err_ioremap:
>         disable_dev = !test_and_set_bit(__E1000_DISABLED, &adapter->flags);
>         free_netdev(netdev);
> --
> 2.47.3
>

Re: [PATCH net v3] net: e1000: fix warning in iounmap on probe failure

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message and v3 changelog describe a different fix
  from the diff that was submitted.
- [Low] The new `if (hw->hw_addr)` check before `iounmap(hw->hw_addr)` in
  e1000_probe() (drivers/net/ethernet/intel/e1000/e1000_main.c:1232) is…
- [Low] The subject quoted in the Fixes tag is wrong.

--

net: e1000: fix warning in iounmap on probe failure

This patch changes the iounmap() calls in the e1000_probe() error unwind
so that a NULL hw->ce4100_gbe_mdio_base_virt is never passed to
iounmap(). On non-CE4100 hardware that pointer stays NULL, and on x86 a
NULL argument to iounmap() triggers a WARN_ON_ONCE.

Closes: https://syzkaller.appspot.com/bug?extid=ca1ef9e2e234b8d3599b

> Fixes: 13acde8fffc0af ("e1000: Fix the CE4100 bus type for the MDIO/PHY registers")

[Severity: Low]
The subject quoted here doesn't match the commit it points to. The title
of 13acde8fffc0af is:

  e1000: cleanup CE4100 MDIO registers access

The SHA itself looks right. That commit added the unconditional
iounmap(hw->ce4100_gbe_mdio_base_virt) to the probe error path.

Could the tag use the real subject and the usual 12 character abbreviated
SHA?

> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index d7f5c6f16142..156903089fa6 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -1227,8 +1227,10 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	kfree(adapter->rx_ring);
>  err_dma:
>  err_sw_init:
> -	iounmap(hw->ce4100_gbe_mdio_base_virt);
> -	iounmap(hw->hw_addr);
> +	if (hw->ce4100_gbe_mdio_base_virt)
> +		iounmap(hw->ce4100_gbe_mdio_base_virt);

[Severity: Medium]
Is the commit message describing this version of the code? The message
says:

    Fix this by conditionally unmapping the CE4100 MDIO base only if the
    mac_type is e1000_ce4100, matching the exact logic used in e1000_remove().
    The hw->hw_addr iounmap() remains unconditional since it is guaranteed to
    be valid for all error paths reaching the err_sw_init label.

The v3 changelog says:

  - Switched from pointer null-checks to checking hw->mac_type == e1000_ce4100
  - Removed redundant check for hw->hw_addr since it cannot be NULL here

The diff has no mac_type check, and iounmap(hw->hw_addr) is still behind
a NULL check. That looks like the v2 approach.

Was the wrong diff sent with the v3 message, or is the message out of
date? The Reviewed-by may also have been given for a different version.

The pointer check in the diff looks like the better choice. A mac_type
check would still reach iounmap(NULL) when the CE4100 ioremap fails in
e1000_probe():

	if (hw->mac_type == e1000_ce4100) {
		hw->ce4100_gbe_mdio_base_virt =
					ioremap(pci_resource_start(pdev, BAR_1),
						pci_resource_len(pdev, BAR_1));

		if (!hw->ce4100_gbe_mdio_base_virt)
			goto err_mdio_ioremap;
	}

err_mdio_ioremap frees the tx and rx rings, then falls through to
err_sw_init. On that path mac_type is e1000_ce4100 but the MDIO pointer
is NULL. A mac_type check would therefore still hit
WARN_ON_ONCE(!is_ioremap_addr(addr)) in the x86 iounmap().

Could the commit message and changelog be updated to describe the NULL
pointer check, instead of changing the code to match the message?

> +	if (hw->hw_addr)
> +		iounmap(hw->hw_addr);

[Severity: Low]
Can hw->hw_addr ever be NULL here? It is mapped earlier in e1000_probe():

	hw->hw_addr = pci_ioremap_bar(pdev, BAR_0);
	if (!hw->hw_addr)
		goto err_ioremap;

The err_ioremap label comes after this block. Every other path into
err_sw_init runs after the mapping has succeeded, so the check is always
true.

The commit message says the hw_addr iounmap() "remains unconditional",
and the v3 changelog says this check was removed. Should it be dropped?

>  err_ioremap:
>  	disable_dev = !test_and_set_bit(__E1000_DISABLED, &adapter->flags);
>  	free_netdev(netdev);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924095152.3069-1-nsvatoslav515%40gmail.com