All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] PCI: Add pci_rebar_size_supported() helper
@ 2025-11-21  9:48 Dan Carpenter
  2025-11-21 11:38 ` Ilpo Järvinen
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2025-11-21  9:48 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: linux-pci

Hello Ilpo Järvinen,

Commit bb1fabd0d94e ("PCI: Add pci_rebar_size_supported() helper")
from Nov 13, 2025 (linux-next), leads to the following Smatch static
checker warning:

	drivers/pci/rebar.c:142 pci_rebar_size_supported()
	error: undefined (user controlled) shift '(((1))) << size'

The problem is this call tree:
__resource_resize_store() <- takes an unsigned long from the user
  -> pci_resize_resource() <- truncates it to int
     -> pci_rebar_size_supported()

drivers/pci/rebar.c
    138 bool pci_rebar_size_supported(struct pci_dev *pdev, int bar, int size)
    139 {
    140         u64 sizes = pci_rebar_get_possible_sizes(pdev, bar);
    141 
--> 142         return BIT(size) & sizes;
    143 }

So here size could be negative or >= BITS_PER_LONG which leads to
shift wrapping.  But also truncating the ulong to int in
__resource_resize_store() is not beautiful.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] PCI: Add pci_rebar_size_supported() helper
  2025-11-21  9:48 [bug report] PCI: Add pci_rebar_size_supported() helper Dan Carpenter
@ 2025-11-21 11:38 ` Ilpo Järvinen
  0 siblings, 0 replies; 2+ messages in thread
From: Ilpo Järvinen @ 2025-11-21 11:38 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-pci

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

On Fri, 21 Nov 2025, Dan Carpenter wrote:

> Hello Ilpo Järvinen,
> 
> Commit bb1fabd0d94e ("PCI: Add pci_rebar_size_supported() helper")
> from Nov 13, 2025 (linux-next), leads to the following Smatch static
> checker warning:
> 
> 	drivers/pci/rebar.c:142 pci_rebar_size_supported()
> 	error: undefined (user controlled) shift '(((1))) << size'
> 
> The problem is this call tree:
> __resource_resize_store() <- takes an unsigned long from the user
>   -> pci_resize_resource() <- truncates it to int
>      -> pci_rebar_size_supported()
> 
> drivers/pci/rebar.c
>     138 bool pci_rebar_size_supported(struct pci_dev *pdev, int bar, int size)
>     139 {
>     140         u64 sizes = pci_rebar_get_possible_sizes(pdev, bar);
>     141 
> --> 142         return BIT(size) & sizes;
>     143 }
> 
> So here size could be negative or >= BITS_PER_LONG which leads to
> shift wrapping.  But also truncating the ulong to int in
> __resource_resize_store() is not beautiful.

Thanks Dan!

I've not liked using int for those size parameters as the field on PCIe 
side is obviously unsigned (less than u8 actually, PCIe r7.0, sec 7.8.6.3) 
but haven't yet spent time on converting them either.

The issue seems older though than introduction of 
pci_rebar_size_supported() in the commit bb1fabd0d94e ("PCI: Add 
pci_rebar_size_supported() helper") that just moved that BIT() inside the 
newly introduced function.

I'll send the fix next week (I wrote it already but they seem to be doing 
some electric work over this weekend so I can't easily do testing for it 
with systems I normally play with BAR resizing).

-- 
 i.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-11-21 11:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-21  9:48 [bug report] PCI: Add pci_rebar_size_supported() helper Dan Carpenter
2025-11-21 11:38 ` Ilpo Järvinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.