Re: [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow

[shao.mingyin]

Hi Ilpo,

Thanks for the review.

> This is missing how it can be reproduced with the existing kernel code.
> If it's not triggerable, the Fixes tag should be dropped.

The only existing path I can point at is device-controlled: sriov_init()
leaves barsz[] zero for unimplemented VF BARs, and a device can point the
VF ReBAR control's BAR_IDX at such a BAR.  sriov_restore_vf_rebar_state()
then calls pci_rebar_bytes_to_size(dev->sriov->barsz[bar_idx]) with zero,
on the sriov_restore_state() path.

That is not a conforming-device path, so I understand if you don't want
to treat it as a Fixes bug.  If so, I'll drop the Fixes tag and keep the
guard as hardening.  The commit message will describe the path and the
TEST-ONLY hook honestly either way; it should have done that from the
start.

The bit-63/U64_MAX inputs are hardening: the largest valid BAR size is
128 TB, so no real resource reaches them.  I'll keep that guard but not
use it to justify Fixes.

> So no real reproducer?

No real-hardware reproducer: QEMU has no rebar model, and the hook only
calls the function directly.  It is a deterministic test for the guard,
not a path-level reproduction.

> Won't this clash with the normal return values? -EINVAL seems better
> choice.
> ...So now you get to add the error handling too.

Agreed; 0 is a valid encoding (1 MB), so returning it for invalid input
is ambiguous.  v2 will return -EINVAL and handle it at the callers:

- pci_restore_rebar_state() and sriov_restore_vf_rebar_state() write
  BAR_SIZE directly, so they will skip the BAR on -EINVAL.
- i915's _resize_bar() and xe's resize_bar() log 1 << bar_size, and
  xe_pci_rebar_resize() also calls pci_rebar_size_to_bytes(); they need
  a negative check before those shifts.
- amdgpu_device_resize_fb_bar() needs the same before using rbar_size.

pci_resize_resource() already rejects a negative size through
pci_rebar_size_supported(), but the direct register writes and log
sites above do not.

Thanks,
Shao