[v1] net: starfire: fix ioaddr sign-extension causing ioremap() failure

[PATCH] net: starfire: fix ioaddr sign-extension causing ioremap() failure

Posted by Ivy Lopez 1 month, 2 weeks ago

ioaddr is declared as a signed long, but is assigned the result of
pci_resource_start(), which returns an unsigned resource_size_t.
On configurations where the BAR address has its high bit set, the
value sign-extends when passed to ioremap(), producing a bogus
64-bit address and causing device probe to fail:

  ioremap: invalid physical address fffffffffe480000
  starfire 0000:08:04.0: cannot Remap 0x80000 @ 0xfe480000, aborting

Change ioaddr to unsigned long so pci_resource_start()'s value is
preserved correctly.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=198035
Signed-off-by: Ivy Lopez <skunkolee@gmail.com>

Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
---
 drivers/net/ethernet/adaptec/starfire.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index f1109d90e1fc..b72b393e6fb2 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -634,7 +634,7 @@ static int starfire_init_one(struct pci_dev *pdev,
 	int i, irq, chip_idx = ent->driver_data;
 	struct net_device *dev;
 	u8 addr[ETH_ALEN];
-	long ioaddr;
+	unsigned long ioaddr;
 	void __iomem *base;
 	int drv_flags, io_size;
 	int boguscnt;
-- 
2.55.0

Re: [PATCH] net: starfire: fix ioaddr sign-extension causing ioremap() failure

Posted by Simon Horman 1 month, 1 week ago

On Sat, Aug 15, 2026 at 01:24:32PM -0600, Ivy Lopez wrote:
> ioaddr is declared as a signed long, but is assigned the result of
> pci_resource_start(), which returns an unsigned resource_size_t.
> On configurations where the BAR address has its high bit set, the
> value sign-extends when passed to ioremap(), producing a bogus
> 64-bit address and causing device probe to fail:
> 
>   ioremap: invalid physical address fffffffffe480000
>   starfire 0000:08:04.0: cannot Remap 0x80000 @ 0xfe480000, aborting
> 
> Change ioaddr to unsigned long so pci_resource_start()'s value is
> preserved correctly.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198035
> Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
> 
> Signed-off-by: Ivy Lopez <skunkolee@gmail.com>

I guess this is a minor tooling mishap, but one Signed-off-by is enough.

> ---
>  drivers/net/ethernet/adaptec/starfire.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
> index f1109d90e1fc..b72b393e6fb2 100644
> --- a/drivers/net/ethernet/adaptec/starfire.c
> +++ b/drivers/net/ethernet/adaptec/starfire.c
> @@ -634,7 +634,7 @@ static int starfire_init_one(struct pci_dev *pdev,
>  	int i, irq, chip_idx = ent->driver_data;
>  	struct net_device *dev;
>  	u8 addr[ETH_ALEN];
> -	long ioaddr;
> +	unsigned long ioaddr;
>  	void __iomem *base;
>  	int drv_flags, io_size;
>  	int boguscnt;

I'm wondering if you considered using resource_size_t (and %pa[p]).
Given that is the both type returned by pci_resource_start() and
the type of the phys_addr parameter to ioremap().

[PATCH v2] net: starfire: fix ioaddr sign-extension causing ioremap() failure

Posted by Ivy Lopez 2 weeks, 6 days ago

ioaddr is declared as a signed long, but is assigned the result of
pci_resource_start(), which returns an unsigned resource_size_t.
On configurations where the BAR address has its high bit set, the
value sign-extends when passed to ioremap(), producing a bogus
64-bit address and causing device probe to fail:

  ioremap: invalid physical address fffffffffe480000
  starfire 0000:08:04.0: cannot Remap 0x80000 @ 0xfe480000, aborting

Change ioaddr to resource_size_t, matching both the return type of
pci_resource_start() and the type ioremap() expects for its physical
address argument, rather than unsigned long, which is not guaranteed
to be wide enough on all configurations. Switch the associated error
print to %pa accordingly.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=198035
Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
---
v2: use resource_size_t instead of unsigned long, and switch the
    error print to %pa, per Simon Horman's review.
---
 drivers/net/ethernet/adaptec/starfire.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index b72b393e6fb2..b2f3998e128b 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -634,7 +634,7 @@ static int starfire_init_one(struct pci_dev *pdev,
 	int i, irq, chip_idx = ent->driver_data;
 	struct net_device *dev;
 	u8 addr[ETH_ALEN];
-	unsigned long ioaddr;
+	resource_size_t ioaddr;
 	void __iomem *base;
 	int drv_flags, io_size;
 	int boguscnt;
@@ -664,8 +664,8 @@ static int starfire_init_one(struct pci_dev *pdev,
 
 	base = ioremap(ioaddr, io_size);
 	if (!base) {
-		dev_err(d, "cannot remap %#x @ %#lx, aborting\n",
-			io_size, ioaddr);
+		dev_err(d, "cannot remap %#x @ %pa, aborting\n",
+			io_size, &ioaddr);
 		goto err_out_free_res;
 	}
 
-- 
2.55.0

Re: [PATCH v2] net: starfire: fix ioaddr sign-extension causing ioremap() failure

Posted by Simon Horman 2 weeks, 6 days ago

On Mon, Sep 07, 2026 at 04:57:48PM -0600, Ivy Lopez wrote:
> ioaddr is declared as a signed long, but is assigned the result of
> pci_resource_start(), which returns an unsigned resource_size_t.
> On configurations where the BAR address has its high bit set, the
> value sign-extends when passed to ioremap(), producing a bogus
> 64-bit address and causing device probe to fail:
> 
>   ioremap: invalid physical address fffffffffe480000
>   starfire 0000:08:04.0: cannot Remap 0x80000 @ 0xfe480000, aborting
> 
> Change ioaddr to resource_size_t, matching both the return type of
> pci_resource_start() and the type ioremap() expects for its physical
> address argument, rather than unsigned long, which is not guaranteed
> to be wide enough on all configurations. Switch the associated error
> print to %pa accordingly.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198035
> Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
> ---
> v2: use resource_size_t instead of unsigned long, and switch the
>     error print to %pa, per Simon Horman's review.

In general this change looks good to me.
But unfortunately it does not apply against net-next.

Please rebase.

-- 
pw-bot: changes-requested

...

[PATCH net-next v3] net: starfire: fix ioaddr sign-extension causing ioremap() failure

Posted by Ivy Lopez 2 weeks, 5 days ago

ioaddr is declared as a signed long, but is assigned the result of
pci_resource_start(), which returns an unsigned resource_size_t.
On configurations where the BAR address has its high bit set, the
value sign-extends when passed to ioremap(), producing a bogus
64-bit address and causing device probe to fail:

  ioremap: invalid physical address fffffffffe480000
  starfire 0000:08:04.0: cannot Remap 0x80000 @ 0xfe480000, aborting

Change ioaddr to resource_size_t, matching both the return type of
pci_resource_start() and the type ioremap() expects for its physical
address argument, rather than unsigned long, which is not guaranteed
to be wide enough on all configurations. Switch the associated error
print to %pa accordingly.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=198035
Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
---
---
v2: use resource_size_t instead of unsigned long, and switch the
    error print to %pa, per Simon Horman's review.
v3: rebase against net-next, no content changes, per Simon Horman.
---
 drivers/net/ethernet/adaptec/starfire.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index f1109d90e1fc..b2f3998e128b 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -634,7 +634,7 @@ static int starfire_init_one(struct pci_dev *pdev,
 	int i, irq, chip_idx = ent->driver_data;
 	struct net_device *dev;
 	u8 addr[ETH_ALEN];
-	long ioaddr;
+	resource_size_t ioaddr;
 	void __iomem *base;
 	int drv_flags, io_size;
 	int boguscnt;
@@ -664,8 +664,8 @@ static int starfire_init_one(struct pci_dev *pdev,
 
 	base = ioremap(ioaddr, io_size);
 	if (!base) {
-		dev_err(d, "cannot remap %#x @ %#lx, aborting\n",
-			io_size, ioaddr);
+		dev_err(d, "cannot remap %#x @ %pa, aborting\n",
+			io_size, &ioaddr);
 		goto err_out_free_res;
 	}
 
-- 
2.55.0

Re: [PATCH net-next v3] net: starfire: fix ioaddr sign-extension causing ioremap() failure

Posted by Simon Horman 2 weeks, 4 days ago

On Tue, Sep 08, 2026 at 06:28:29PM -0600, Ivy Lopez wrote:
> ioaddr is declared as a signed long, but is assigned the result of
> pci_resource_start(), which returns an unsigned resource_size_t.
> On configurations where the BAR address has its high bit set, the
> value sign-extends when passed to ioremap(), producing a bogus
> 64-bit address and causing device probe to fail:
> 
>   ioremap: invalid physical address fffffffffe480000
>   starfire 0000:08:04.0: cannot Remap 0x80000 @ 0xfe480000, aborting
> 
> Change ioaddr to resource_size_t, matching both the return type of
> pci_resource_start() and the type ioremap() expects for its physical
> address argument, rather than unsigned long, which is not guaranteed
> to be wide enough on all configurations. Switch the associated error
> print to %pa accordingly.

Sorry for not noticing this earlier, but probably this should
have a Fixes tag. No need to repost just for this but I think
it should be:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=198035
> Signed-off-by: Ivy Lopez <skunkolee@gmail.com>

Reviewed-by: Simon Horman <horms@kernel.org>

> ---
> ---
> v2: use resource_size_t instead of unsigned long, and switch the
>     error print to %pa, per Simon Horman's review.
> v3: rebase against net-next, no content changes, per Simon Horman.

...