[PATCH v2] PCI: mediatek: Fix integer truncation in fls() and clamp size

[PATCH v2] PCI: mediatek: Fix integer truncation in fls() and clamp size

[Sreeraj S Kurup]

resource_size() returns a resource_size_t, which is 64-bit on 64-bit
architectures or 32-bit systems with LPAE/PAE enabled. Passing this
directly to fls(), which accepts an unsigned int, implicitly
truncates the upper 32 bits.

Furthermore, AHB2PCIE_SIZE() uses a 5-bit mask GENMASK(4, 0). If a
resource size of 4 GiB or larger is passed, fls64() returns 33 or
greater, which overflows the 5-bit mask and wraps around (e.g. 33 & 31
= 1).

Fix this by using fls64() for 64-bit resource sizes and clamping the
result to a maximum of 31 to fit the 5-bit register field.

Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
---
 drivers/pci/controller/pcie-mediatek.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index a60d1ae076f8..9576fe532b92 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
@@ -686,6 +687,8 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 	const struct mtk_pcie_soc *soc = port->pcie->soc;
 	u32 val;
 	int err;
+	resource_size_t size;
+	int size_order;
 
 	entry = resource_list_first_type(&host->windows, IORESOURCE_MEM);
 	if (entry)
@@ -753,8 +756,13 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 		mtk_pcie_enable_msi(port);
 
 	/* Set AHB to PCIe translation windows */
+	size = resource_size(mem);
+	size_order = fls64(size);
+	if (size_order > 31)
+		size_order = 31;
+
 	val = lower_32_bits(mem->start) |
-	      AHB2PCIE_SIZE(fls(resource_size(mem)));
+	      AHB2PCIE_SIZE(size_order);
 	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
 
 	val = upper_32_bits(mem->start);
@@ -775,6 +783,8 @@ static int mtk_pcie_startup_port_en7528(struct mtk_pcie_port *port)
 	struct resource_entry *entry;
 	u32 val, link_mask;
 	int err;
+	resource_size_t size;
+	int size_order;
 
 	entry = resource_list_first_type(&host->windows, IORESOURCE_MEM);
 	if (entry)
@@ -829,8 +839,13 @@ static int mtk_pcie_startup_port_en7528(struct mtk_pcie_port *port)
 		mtk_pcie_enable_msi(port);
 
 	/* Set AHB to PCIe translation windows */
+	size = resource_size(mem);
+	size_order = fls64(size);
+	if (size_order > 31)
+		size_order = 31;
+
 	val = lower_32_bits(mem->start) |
-	      AHB2PCIE_SIZE(fls(resource_size(mem)));
+	      AHB2PCIE_SIZE(size_order);
 	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
 
 	val = upper_32_bits(mem->start);
-- 
2.55.0

Re: [PATCH v2] PCI: mediatek: Fix integer truncation in fls() and clamp size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: silent hardware window truncation causes unmapped BARs
--

commit 3807ae15ec6ca796b24e06b510d2cb2153d9a5c8
Author: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

PCI: mediatek: Fix integer truncation in fls() and clamp size

This commit fixes an integer truncation issue by using fls64() for 64-bit
resource sizes. It also clamps the resulting size_order to a maximum of 31
to avoid overflowing the 5-bit register field used in AHB2PCIE_SIZE().

> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 3a3e91cd0c211..ddc7ed28ef843 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
[ ... ]
> @@ -753,8 +756,13 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> +	size = resource_size(mem);
> +	size_order = fls64(size);
> +	if (size_order > 31)
> +		size_order = 31;
> +

[Severity: High]
Does clamping the size order here in mtk_pcie_startup_port_v2() cause
unmapped BARs?

The hardware translation window size is clamped to size order 31, but the
patch does not adjust the OS-level mem resource (e.g., mem->end). If the
device tree provides a PCIe memory resource of 4 GiB or larger, the Linux
PCI core will allocate BARs within the full original window. Because the
hardware only translates the truncated portion, will CPU accesses to BARs
placed above the clamped limit fail with a data abort or bus error?

Additionally, using fls64(size) on a power-of-two size computes
ilog2(size) + 1, which may evaluate to a larger window than requested.

Should the driver reject oversized windows entirely, or shrink the OS
resource size to match the hardware reality?

>  	val = lower_32_bits(mem->start) |
> -	      AHB2PCIE_SIZE(fls(resource_size(mem)));
> +	      AHB2PCIE_SIZE(size_order);
>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);
[ ... ]
> @@ -829,8 +839,13 @@ static int mtk_pcie_startup_port_en7528(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> +	size = resource_size(mem);
> +	size_order = fls64(size);
> +	if (size_order > 31)
> +		size_order = 31;
> +

[Severity: High]
Does this suffer from the same silent hardware window truncation issue as
mtk_pcie_startup_port_v2() above? The hardware translation window is
clamped without adjusting the corresponding OS-level resource.

>  	val = lower_32_bits(mem->start) |
> -	      AHB2PCIE_SIZE(fls(resource_size(mem)));
> +	      AHB2PCIE_SIZE(size_order);
>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930105435.3660-1-sreekuttan2156239@gmail.com?part=1