mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference
@ 2026-11-06 16:23 shao.mingyin
  2026-11-06 16:23 ` [PATCH 1/5] PCI: Reject negative resource alignment order shao.mingyin
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 16:23 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: mani, kwilczynski, lpieralisi, kishon, Frank.Li, linux-kernel

Several PCI paths consume values from the command line, a device tree,
config space or an endpoint driver without validating them, and the
ASPM link setup does not handle a downstream bus without function 0.
The five fixes reject the bad values, or skip the unsafe state, before
they reach a shift, an array index or a pointer dereference.

The series is ordered by severity and is independent patch-by-patch;
each patch can be applied and reviewed on its own, and taking a subset
is fine:

  1/5 PCI: Reject negative resource alignment order
  2/5 PCI: of: Bound num_lanes in of_pci_get_equalization_presets()
  3/5 PCI/ASPM: Skip link-state setup without downstream function 0
  4/5 PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow
  5/5 PCI: endpoint: Reject a zero interrupt number in raise_irq

1/5 rejects a negative pci=resource_alignment= order, 2/5 rejects a
num_lanes value larger than the 16-entry preset arrays, 3/5 skips ASPM
link-state setup when the downstream bus has no function 0, 4/5 rejects
resizable BAR sizes that cannot be encoded, and 5/5 rejects a zero
MSI/MSI-X interrupt number before it underflows in the controller.

All five are single-file changes under drivers/pci/ and pass git apply
and checkpatch.  Each patch was tested on 7.3.0-next-20261030 in a
QEMU/TCG guest with an unpatched/patched A/B comparison; the test
section in each patch describes the setup and the observed results.

Shao Mingyin (5):
  PCI: Reject negative resource alignment order
  PCI: of: Bound num_lanes in of_pci_get_equalization_presets()
  PCI/ASPM: Skip link-state setup without downstream function 0
  PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow
  PCI: endpoint: Reject a zero interrupt number in raise_irq

 drivers/pci/endpoint/pci-epc-core.c | 4 ++++
 drivers/pci/of.c                    | 3 +++
 drivers/pci/pci.c                   | 2 +-
 drivers/pci/pcie/aspm.c             | 3 +++
 drivers/pci/rebar.c                 | 7 +++++++
 5 files changed, 18 insertions(+), 1 deletion(-)

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

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

* [PATCH 1/5] PCI: Reject negative resource alignment order
  2026-11-06 16:23 [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference shao.mingyin
@ 2026-11-06 16:23 ` shao.mingyin
  2026-11-06 16:39   ` Ilpo Järvinen
  2026-11-06 16:23 ` [PATCH 2/5] PCI: of: Bound num_lanes in of_pci_get_equalization_presets() shao.mingyin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 16:23 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: mani, kwilczynski, lpieralisi, kishon, Frank.Li, linux-kernel

From: Shao Mingyin <shao.mingyin@zte.com.cn>

pci_specified_resource_alignment() parses the pci=resource_alignment=
boot parameter with sscanf("%d%n", &align_order, ...) and rejects only
orders above 63.  A negative order, e.g. "pci=resource_alignment=-5@...",
passes that check and later reaches "align = 1ULL << align_order" with a
negative shift count, which is undefined behavior; on x86 the 64-bit
shift count is masked to six bits, silently producing a 2^59-byte
alignment that mis-aligns (or fails to align) the affected IOV resources.

Reject any order outside the valid [0, 63] range and fall back to the
default page-size alignment, matching the existing handling of
out-of-range positive orders.

Fixes: 6534aac198b5 ("PCI: Bounds-check command-line resource alignment requests")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the
pci=resource_alignment= boot parameter and a QEMU e1000 NIC; no extra
hardware is needed.

Booting with pci=resource_alignment=-5@pci:8086:100e parses a negative
order that the "> 63" check lets through, so "align = 1ULL << align_order"
shifts by -5 (masked to 2^59 on x86-64); the e1000 BAR is resized to
2^59 bytes, which cannot be assigned, and the e1000 fails to probe:

  unpatched                            patched
  requesting alignment to              PCI: Invalid requested alignment
  0x800000000000000                    (order -5)
  e1000 probe failed, no eth0          e1000 eth0 probes

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index bca3a5d84cd9..c2260ee1f956 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6605,7 +6605,7 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
 		if (sscanf(p, "%d%n", &align_order, &count) == 1 &&
 		    p[count] == '@') {
 			p += count + 1;
-			if (align_order > 63) {
+			if (align_order < 0 || align_order > 63) {
 				pr_err("PCI: Invalid requested alignment (order %d)\n",
 				       align_order);
 				align_order = PAGE_SHIFT;

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

* [PATCH 2/5] PCI: of: Bound num_lanes in of_pci_get_equalization_presets()
  2026-11-06 16:23 [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference shao.mingyin
  2026-11-06 16:23 ` [PATCH 1/5] PCI: Reject negative resource alignment order shao.mingyin
@ 2026-11-06 16:23 ` shao.mingyin
  2026-11-06 16:23 ` [PATCH 3/5] PCI/ASPM: Skip link-state setup without downstream function 0 shao.mingyin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 16:23 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: mani, kwilczynski, lpieralisi, kishon, Frank.Li, linux-kernel

From: Shao Mingyin <shao.mingyin@zte.com.cn>

of_pci_get_equalization_presets() copies num_lanes entries into
presets->eq_presets_8gts[] and presets->eq_presets_Ngts[i][], which are
both sized MAX_NR_LANES (16).  The only in-tree caller,
dw_pcie_host_init(), passes pci->num_lanes, which comes from the
"num-lanes" DT property or from the Link Capabilities MLW field (up to 63
for PCIe r6.0), so a device advertising more than 16 lanes makes
of_property_read_*_array() write past the end of those arrays.
Return -EINVAL when num_lanes exceeds MAX_NR_LANES instead of overflowing
the output arrays.  Callers below the 16-lane limit are unaffected.

Fixes: 57a4591df709 ("PCI: of: Add of_pci_get_equalization_presets() API")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest, KASAN) with a synthetic
device-tree node carrying 32-entry "eq-presets-*" properties, passed to
of_pci_get_equalization_presets().  No PCIe controller hardware is needed:
a TEST-ONLY hook (applied to both kernels) builds the node and calls the
function once at boot with num_lanes=32.

Calling the function with num_lanes=32 copies 32 entries into the
MAX_NR_LANES (16) eq_presets output arrays:

  unpatched                            patched
  BUG: KASAN: stack-out-of-bounds      no KASAN report, ret=-EINVAL
  in of_property_read_variable_u8_     (num_lanes > 16 rejected up front)
  array, of_pci_get_equalization_
  presets+0x181, ret=0

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index a51dff91b196..85737043294e 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -1062,6 +1062,9 @@ int of_pci_get_equalization_presets(struct device *dev,
 	char name[20];
 	int ret;
 
+	if (num_lanes > MAX_NR_LANES)
+		return -EINVAL;
+
 	presets->eq_presets_8gts[0] = PCI_EQ_RESV;
 	ret = of_property_read_u16_array(dev->of_node, "eq-presets-8gts",
 					 presets->eq_presets_8gts, num_lanes);

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

* [PATCH 3/5] PCI/ASPM: Skip link-state setup without downstream function 0
  2026-11-06 16:23 [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference shao.mingyin
  2026-11-06 16:23 ` [PATCH 1/5] PCI: Reject negative resource alignment order shao.mingyin
  2026-11-06 16:23 ` [PATCH 2/5] PCI: of: Bound num_lanes in of_pci_get_equalization_presets() shao.mingyin
@ 2026-11-06 16:23 ` shao.mingyin
  2026-11-06 16:23 ` [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow shao.mingyin
  2026-11-06 16:23 ` [PATCH 5/5] PCI: endpoint: Reject a zero interrupt number in raise_irq shao.mingyin
  4 siblings, 0 replies; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 16:23 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: mani, kwilczynski, lpieralisi, kishon, Frank.Li, linux-kernel

From: Shao Mingyin <shao.mingyin@zte.com.cn>

pcie_aspm_init_link_state() builds a link state for a downstream port
whose subordinate bus is non-empty.  For a bridge whose bus has devices
but no function 0 -- a multi-function device with function 0 hidden, or
functions isolated by a hypervisor -- pci_function_0() returns NULL, so
alloc_pcie_link_state() sets link->downstream to NULL.

pcie_aspm_cap_init() then dereferences child->aspm_l0s_support, and even
if that site is guarded the same NULL link->downstream is dereferenced
again by pcie_config_aspm_link() (reached through pcie_config_aspm_path())
and by pcie_aspm_check_latency(), so the link cannot be safely handled at
all.

Skip the whole link-state setup when the downstream bus has no function 0;
nothing below (capability init, latency check, link configuration) then
sees a NULL downstream.

Fixes: 3bd7db63a841 ("PCI/ASPM: Always set link->downstream to avoid NULL dereference on remove")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG x86_64, q35) with a PCIe root port
whose downstream would have devices but no function 0, emulated by a
TEST-ONLY hook (applied to both kernels) that makes pci_function_0()
return NULL for the QEMU root port; QEMU cannot model a function without
its function 0.  No extra hardware is needed.

Unpatched, pcie_aspm_cap_init() dereferences child->aspm_l0s_support with
link->downstream == NULL and the kernel panics.  A guard placed only in
pcie_aspm_cap_init() is not sufficient: with the default (performance)
policy the same NULL is dereferenced again in pcie_config_aspm_link() via
pcie_config_aspm_path().  With this patch pcie_aspm_init_link_state()
skips link-state setup entirely and the kernel boots cleanly, the endpoint
probes normally and the guest powers off.

  unpatched                               patched
  BUG: kernel NULL pointer dereference,   no fault; guest powers off
  address: 0xc0 (aspm_l0s_support)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index b039b57f8466..003b13db5850 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1243,6 +1243,9 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 	if (list_empty(&pdev->subordinate->devices))
 		goto out;
 
+	if (!pci_function_0(pdev->subordinate))
+		goto out;
+
 	mutex_lock(&aspm_lock);
 	link = alloc_pcie_link_state(pdev);
 	if (!link)

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

* [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow
  2026-11-06 16:23 [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference shao.mingyin
                   ` (2 preceding siblings ...)
  2026-11-06 16:23 ` [PATCH 3/5] PCI/ASPM: Skip link-state setup without downstream function 0 shao.mingyin
@ 2026-11-06 16:23 ` shao.mingyin
  2026-11-06 16:37   ` Ilpo Järvinen
  2026-11-06 16:23 ` [PATCH 5/5] PCI: endpoint: Reject a zero interrupt number in raise_irq shao.mingyin
  4 siblings, 1 reply; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 16:23 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: mani, kwilczynski, lpieralisi, kishon, Frank.Li, linux-kernel

From: Shao Mingyin <shao.mingyin@zte.com.cn>

pci_rebar_bytes_to_size() passes its argument straight to
roundup_pow_of_two(), whose implementation is (1UL << fls(bytes - 1)).
For bytes == 0 and for values above 2^63, fls() returns 64 and the
shift count is undefined behavior.  A value with bit 63 set cannot be
encoded either: exactly 2^63 rounds to itself and is encoded as 43,
outside the 5-bit BAR Size field.

Bail out for bytes == 0 and for any value with bit 63 set; a zero size
is returned, which encodes the 1 MB minimum.  The largest valid BAR size
is 128 TB, far below the 2^63 boundary, so no legitimate input is
affected.

Fixes: 192f1bf7559e ("PCI: Add pci_rebar_bytes_to_size()")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG x86_64) with a TEST-ONLY hook
(applied to both kernels) that calls pci_rebar_bytes_to_size() at boot
with pathological inputs (0, 2^63, U64_MAX) and two sane sizes (1 MB,
1 GB), built with CONFIG_UBSAN_SHIFT.  No extra hardware is needed.

Unpatched, the 2^63 input is encoded as BAR size 43 (outside the valid
0..31 range) and the 0/U64_MAX inputs shift by 64 (UBSAN reports "shift
exponent 64 is too large for 64-bit type").  With this patch all
pathological inputs return 0, the two sane sizes are unchanged (1 MB -> 0,
1 GB -> 10) and no UBSAN report appears.

  unpatched                               patched
  UBSAN shift-out-of-bounds; size 43      size 0, no UBSAN report

diff --git a/drivers/pci/rebar.c b/drivers/pci/rebar.c
index 5bbdc9470a31..fd448dc7c322 100644
--- a/drivers/pci/rebar.c
+++ b/drivers/pci/rebar.c
@@ -31,6 +31,13 @@ int pci_rebar_bytes_to_size(u64 bytes)
 {
 	int rebar_minsize = ilog2(PCI_REBAR_MIN_SIZE);
 
+	/*
+	 * roundup_pow_of_two() is undefined for 0 and above 2^63; any value
+	 * with bit 63 set is outside the 5-bit BAR Size field, so bail out.
+	 */
+	if (!bytes || bytes & (1ULL << 63))
+		return 0;
+
 	bytes = roundup_pow_of_two(bytes);
 
 	return max(ilog2(bytes), rebar_minsize) - rebar_minsize;

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

* [PATCH 5/5] PCI: endpoint: Reject a zero interrupt number in raise_irq
  2026-11-06 16:23 [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference shao.mingyin
                   ` (3 preceding siblings ...)
  2026-11-06 16:23 ` [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow shao.mingyin
@ 2026-11-06 16:23 ` shao.mingyin
  4 siblings, 0 replies; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 16:23 UTC (permalink / raw)
  To: bhelgaas, linux-pci
  Cc: mani, kwilczynski, lpieralisi, kishon, Frank.Li, linux-kernel

From: Shao Mingyin <shao.mingyin@zte.com.cn>

pci_epc_raise_irq() passes interrupt_num straight through to the
controller's raise_irq callback.  The DWC and other controllers compute
"msg_data | (interrupt_num - 1)" for MSI/MSI-X, so an interrupt_num of 0
underflows to 65535 and raises a message with a bogus vector that no
configured MSI/MSI-X entry matches.

Reject interrupt_num == 0 for the MSI and MSI-X types in the core API.
INTx raises use interrupt_num 0 and are unaffected.

Fixes: 5e8cb4033807 ("PCI: endpoint: Add EP core layer to enable EP controller and EP functions")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest).  QEMU has no PCI EP
controller model, so a TEST-ONLY hook (applied to both kernels, not part
of this patch) fabricates a pci_epc whose ops->raise_irq records whether
it is called, then drives pci_epc_raise_irq() with interrupt_num 0 for
MSI and MSI-X.  Without this fix both reach the controller callback (the
DWC style "msg_data | (interrupt_num - 1)" would underflow to 65535);
with this fix both are rejected with -EINVAL while INTx (interrupt_num 0)
still reaches the callback unchanged.

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 831b40458dcd..d8e5dd6355ac 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -295,6 +295,10 @@ int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
 		return -EINVAL;
 
+	if ((type == PCI_IRQ_MSI || type == PCI_IRQ_MSIX) &&
+	    !interrupt_num)
+		return -EINVAL;
+
 	if (!epc->ops->raise_irq)
 		return 0;

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

* Re: [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow
  2026-11-06 16:23 ` [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow shao.mingyin
@ 2026-11-06 16:37   ` Ilpo Järvinen
  2026-11-06 17:11     ` shao.mingyin
  0 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2026-11-06 16:37 UTC (permalink / raw)
  To: shao.mingyin
  Cc: bhelgaas, linux-pci, mani, kwilczynski, lpieralisi, kishon,
	Frank.Li, LKML

On Sat, 7 Nov 2026, shao.mingyin@zte.com.cn wrote:

> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> pci_rebar_bytes_to_size() passes its argument straight to
> roundup_pow_of_two(), whose implementation is (1UL << fls(bytes - 1)).
> For bytes == 0 and for values above 2^63, fls() returns 64 and the
> shift count is undefined behavior.  A value with bit 63 set cannot be
> encoded either: exactly 2^63 rounds to itself and is encoded as 43,
> outside the 5-bit BAR Size field.
> 
> Bail out for bytes == 0 and for any value with bit 63 set; a zero size
> is returned, which encodes the 1 MB minimum.  The largest valid BAR size
> is 128 TB, far below the 2^63 boundary, so no legitimate input is
> affected.
> 
> Fixes: 192f1bf7559e ("PCI: Add pci_rebar_bytes_to_size()")

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

> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> Tested on 7.3.0-next-20261030 (QEMU/TCG x86_64) with a TEST-ONLY hook
> (applied to both kernels) that calls pci_rebar_bytes_to_size() at boot

So no real reproducer?

> with pathological inputs (0, 2^63, U64_MAX) and two sane sizes (1 MB,
> 1 GB), built with CONFIG_UBSAN_SHIFT.  No extra hardware is needed.
> 
> Unpatched, the 2^63 input is encoded as BAR size 43 (outside the valid
> 0..31 range) and the 0/U64_MAX inputs shift by 64 (UBSAN reports "shift
> exponent 64 is too large for 64-bit type").  With this patch all
> pathological inputs return 0, the two sane sizes are unchanged (1 MB -> 0,
> 1 GB -> 10) and no UBSAN report appears.
> 
>   unpatched                               patched
>   UBSAN shift-out-of-bounds; size 43      size 0, no UBSAN report
> 
> diff --git a/drivers/pci/rebar.c b/drivers/pci/rebar.c
> index 5bbdc9470a31..fd448dc7c322 100644
> --- a/drivers/pci/rebar.c
> +++ b/drivers/pci/rebar.c
> @@ -31,6 +31,13 @@ int pci_rebar_bytes_to_size(u64 bytes)
>  {
>  	int rebar_minsize = ilog2(PCI_REBAR_MIN_SIZE);
>  
> +	/*
> +	 * roundup_pow_of_two() is undefined for 0 and above 2^63; any value
> +	 * with bit 63 set is outside the 5-bit BAR Size field, so bail out.
> +	 */
> +	if (!bytes || bytes & (1ULL << 63))
> +		return 0;

Won't this clash with the normal return values? -EINVAL seems better 
choice.

...So now you get to add the error handling too.

-- 
 i.

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

* Re: [PATCH 1/5] PCI: Reject negative resource alignment order
  2026-11-06 16:23 ` [PATCH 1/5] PCI: Reject negative resource alignment order shao.mingyin
@ 2026-11-06 16:39   ` Ilpo Järvinen
  2026-11-06 17:13     ` shao.mingyin
  0 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2026-11-06 16:39 UTC (permalink / raw)
  To: shao.mingyin
  Cc: bhelgaas, linux-pci, mani, kwilczynski, lpieralisi, kishon,
	Frank.Li, LKML

On Sat, 7 Nov 2026, shao.mingyin@zte.com.cn wrote:

> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> pci_specified_resource_alignment() parses the pci=resource_alignment=
> boot parameter with sscanf("%d%n", &align_order, ...) and rejects only
> orders above 63.  A negative order, e.g. "pci=resource_alignment=-5@...",
> passes that check and later reaches "align = 1ULL << align_order" with a
> negative shift count, which is undefined behavior; on x86 the 64-bit
> shift count is masked to six bits, silently producing a 2^59-byte
> alignment that mis-aligns (or fails to align) the affected IOV resources.
> 
> Reject any order outside the valid [0, 63] range and fall back to the
> default page-size alignment, matching the existing handling of
> out-of-range positive orders.
> 
> Fixes: 6534aac198b5 ("PCI: Bounds-check command-line resource alignment requests")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the
> pci=resource_alignment= boot parameter and a QEMU e1000 NIC; no extra
> hardware is needed.
> 
> Booting with pci=resource_alignment=-5@pci:8086:100e parses a negative
> order that the "> 63" check lets through, so "align = 1ULL << align_order"
> shifts by -5 (masked to 2^59 on x86-64); the e1000 BAR is resized to
> 2^59 bytes, which cannot be assigned, and the e1000 fails to probe:
> 
>   unpatched                            patched
>   requesting alignment to              PCI: Invalid requested alignment
>   0x800000000000000                    (order -5)
>   e1000 probe failed, no eth0          e1000 eth0 probes
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index bca3a5d84cd9..c2260ee1f956 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6605,7 +6605,7 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
>  		if (sscanf(p, "%d%n", &align_order, &count) == 1 &&
>  		    p[count] == '@') {
>  			p += count + 1;
> -			if (align_order > 63) {
> +			if (align_order < 0 || align_order > 63) {

Wouldn't it be better to use %u ?

>  				pr_err("PCI: Invalid requested alignment (order %d)\n",
>  				       align_order);
>  				align_order = PAGE_SHIFT;
> 

-- 
 i.


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

* Re: [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow
  2026-11-06 16:37   ` Ilpo Järvinen
@ 2026-11-06 17:11     ` shao.mingyin
  0 siblings, 0 replies; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 17:11 UTC (permalink / raw)
  To: ilpo.jarvinen
  Cc: bhelgaas, linux-pci, mani, kwilczynski, lpieralisi, kishon,
	Frank.Li, linux-kernel

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

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

* Re: [PATCH 1/5] PCI: Reject negative resource alignment order
  2026-11-06 16:39   ` Ilpo Järvinen
@ 2026-11-06 17:13     ` shao.mingyin
  0 siblings, 0 replies; 10+ messages in thread
From: shao.mingyin @ 2026-11-06 17:13 UTC (permalink / raw)
  To: ilpo.jarvinen
  Cc: bhelgaas, linux-pci, mani, kwilczynski, lpieralisi, kishon,
	Frank.Li, linux-kernel

On Fri, 6 Nov 2026, Ilpo Järvinen wrote:
> Wouldn't it be better to use %u ?

Yes, that's cleaner.  With %u the "-5" is converted to 4294967291 and
the existing > 63 check rejects it, so the explicit < 0 test is not
needed.  The error message will print 4294967291 instead of -5; I'll
update it to %u as well.  I'll fold this into v2.

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

end of thread, other threads:[~2026-11-06 17:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06 16:23 [PATCH 0/5] PCI: Fix invalid input handling and a NULL dereference shao.mingyin
2026-11-06 16:23 ` [PATCH 1/5] PCI: Reject negative resource alignment order shao.mingyin
2026-11-06 16:39   ` Ilpo Järvinen
2026-11-06 17:13     ` shao.mingyin
2026-11-06 16:23 ` [PATCH 2/5] PCI: of: Bound num_lanes in of_pci_get_equalization_presets() shao.mingyin
2026-11-06 16:23 ` [PATCH 3/5] PCI/ASPM: Skip link-state setup without downstream function 0 shao.mingyin
2026-11-06 16:23 ` [PATCH 4/5] PCI: rebar: Guard pci_rebar_bytes_to_size() against overflow shao.mingyin
2026-11-06 16:37   ` Ilpo Järvinen
2026-11-06 17:11     ` shao.mingyin
2026-11-06 16:23 ` [PATCH 5/5] PCI: endpoint: Reject a zero interrupt number in raise_irq shao.mingyin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®