[PATCH] PCI: Fix incorrect retval type in remove_id_store()

[PATCH] PCI: Fix incorrect retval type in remove_id_store()

[Alok Tiwari]

remove_id_store() initializes retval with -ENODEV, but retval was
declared as size_t. Since size_t is unsigned, the negative value is
converted to a large positive number, breaking error handling.

Use ssize_t to match the sysfs store() return type.

Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
 drivers/pci/pci-driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index dd9075403987..b2d3dffcc4f3 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -261,7 +261,7 @@ static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
 	u32 vendor, device, subvendor = PCI_ANY_ID,
 		subdevice = PCI_ANY_ID, class = 0, class_mask = 0;
 	int fields;
-	size_t retval = -ENODEV;
+	ssize_t retval = -ENODEV;
 
 	fields = sscanf(buf, "%x %x %x %x %x %x",
 			&vendor, &device, &subvendor, &subdevice,
-- 
2.50.1

Re: [PATCH] PCI: Fix incorrect retval type in remove_id_store()

[Bjorn Helgaas]

On Tue, Feb 24, 2026 at 10:14:29AM -0800, Alok Tiwari wrote:
> remove_id_store() initializes retval with -ENODEV, but retval was
> declared as size_t. Since size_t is unsigned, the negative value is
> converted to a large positive number, breaking error handling.

Does this actually cause a problem?  size_t and ssize_t are the same
size, so I would think the conversions would end up with the same
bits:

  (gdb) p (long)((unsigned long) -19)
  $1 = -19
  (gdb) p (int)((unsigned int) -19)
  $2 = -19

We have lots of sysfs show/store functions in pci-sysfs.c that return
size_t values.

> Use ssize_t to match the sysfs store() return type.
> 
> Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
>  drivers/pci/pci-driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index dd9075403987..b2d3dffcc4f3 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -261,7 +261,7 @@ static ssize_t remove_id_store(struct device_driver *driver, const char *buf,
>  	u32 vendor, device, subvendor = PCI_ANY_ID,
>  		subdevice = PCI_ANY_ID, class = 0, class_mask = 0;
>  	int fields;
> -	size_t retval = -ENODEV;
> +	ssize_t retval = -ENODEV;
>  
>  	fields = sscanf(buf, "%x %x %x %x %x %x",
>  			&vendor, &device, &subvendor, &subdevice,
> -- 
> 2.50.1
> 

Re: [PATCH] PCI: Fix incorrect retval type in remove_id_store()

[ALOK TIWARI]

Thanks Bjorn,

On 3/10/2026 3:50 AM, Bjorn Helgaas wrote:
> On Tue, Feb 24, 2026 at 10:14:29AM -0800, Alok Tiwari wrote:
>> remove_id_store() initializes retval with -ENODEV, but retval was
>> declared as size_t. Since size_t is unsigned, the negative value is
>> converted to a large positive number, breaking error handling.
> Does this actually cause a problem?  size_t and ssize_t are the same
> size, so I would think the conversions would end up with the same
> bits:
> 
>    (gdb) p (long)((unsigned long) -19)
>    $1 = -19
>    (gdb) p (int)((unsigned int) -19)
>    $2 = -19
> 
> We have lots of sysfs show/store functions in pci-sysfs.c that return
> size_t values.

Looking through pci_sysfs.c, the sysfs handlers consistently return 
ssize_t, and negative error codes are returned directly.
When intermediate variables are used, they are typically ssize_t or int 
(for example, driver_override_store()
uses int ret, and driver_override_show() uses ssize_t len).

size_t seems to be used only for byte counts like count or buffer 
lengths, not for storing error codes.
So using ssize_t here would match the existing pattern and avoid 
assigning negative errors to an unsigned type.

> 
>> Use ssize_t to match the sysfs store() return type.
>>
>> Signed-off-by: Alok Tiwari<alok.a.tiwari@oracle.com>
>> ---
>>   drivers/pci/pci-driver.c | 2 +-


Thanks,
Alok

Re: [PATCH] PCI: Fix incorrect retval type in remove_id_store()

[Bjorn Helgaas]

On Tue, Mar 10, 2026 at 08:36:25AM +0530, ALOK TIWARI wrote:
> On 3/10/2026 3:50 AM, Bjorn Helgaas wrote:
> > On Tue, Feb 24, 2026 at 10:14:29AM -0800, Alok Tiwari wrote:
> > > remove_id_store() initializes retval with -ENODEV, but retval was
> > > declared as size_t. Since size_t is unsigned, the negative value is
> > > converted to a large positive number, breaking error handling.
> >
> > Does this actually cause a problem?

Can you answer this question?  I think the patch itself is OK, but I
don't want to claim that it fixes broken error handling unless there's
actually some way to observe that broken behavior.

Re: [PATCH] PCI: Fix incorrect retval type in remove_id_store()

[ALOK TIWARI]

On 3/10/2026 9:33 PM, Bjorn Helgaas wrote:
>>> Does this actually cause a problem?

No this doesn’t fix broken error handling. The behavior is unchanged

We can note: "No functional change intended."

> Can you answer this question?  I think the patch itself is OK, but I
> don't want to claim that it fixes broken error handling unless there's
> actually some way to observe that broken behavior.


Thanks,
Alok