Re: [PATCH v5 3/3] arm64: dts: qcom: eliza-evk: Add PCIe1 with TC9563 PCIe switch

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Buffer overflow in the `pwrctrl-tc9563` driver triggered by the addition of the `pcie@0,1` node.
- [Medium] The Ethernet endpoint nodes `pcie@0,0` and `pcie@0,1` under downstream port `pcie@3,0` incorrectly specify PCI bridge properties.
- [Low] Misleading comment regarding shared GPIO pins.
--

--- Patch [3]: [PATCH v5 3/3] arm64: dts: qcom: eliza-evk: Add PCIe1 with TC9563 PCIe switch ---
commit afe372355fdb42a2ae6e1af16238819812ac2af7
Author: Krishna Chaitanya Chundru <[email protected]>

arm64: dts: qcom: eliza-evk: Add PCIe1 with TC9563 PCIe switch

The Eliza EVK board connects PCIe1 (8GT/s x2) to a Toshiba TC9563
PCIe switch. Enable PCIe1 and its QMP PHY nodes.

> diff --git a/arch/arm64/boot/dts/qcom/eliza-evk.dtsi b/arch/arm64/boot/dts/qcom/eliza-evk.dtsi
> index b35fffb509765..419e737c54e6f 100644
> --- a/arch/arm64/boot/dts/qcom/eliza-evk.dtsi
> +++ b/arch/arm64/boot/dts/qcom/eliza-evk.dtsi
[ ... ]
> @@ -99,6 +119,99 @@ pcieport0_ep: endpoint {
>  	};
>  };
>  
[ ... ]
> +&pcie1_port0 {
> +	wake-gpios = <&tlmm 53 GPIO_ACTIVE_LOW>;
> +	/* Same GPIO is used for both PERST# and resx-gpio */

[Severity: Low]
This isn't a bug, but this comment states that the same GPIO is used for both
PERST# and resx-gpio, however resx-gpios is assigned to GPIO 54 below.

Should this comment be updated or moved?

> +
> +	tc9563: pcie@0,0 {
> +		compatible = "pci1179,0623";
[ ... ]
> +		resx-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>;
> +
[ ... ]
> +		pcie@3,0 {
> +			reg = <0x21800 0x0 0x0 0x0 0x0>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			device_type = "pci";
> +			ranges;
> +			bus-range = <0x5 0xff>;
> +
> +			pcie@0,0 {
> +				reg = <0x50000 0x0 0x0 0x0 0x0>;
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				ranges;
> +			};

[Severity: Medium]
Do these integrated Ethernet MAC endpoints incorrectly specify PCI bridge
properties?

The OF framework may parse device_type = "pci", ranges, #address-cells, and
#size-cells on endpoint nodes during boot, which can cause the PCI core to
misidentify them as PCI-to-PCI bridges.

> +
> +			pcie@0,1 {
> +				reg = <0x50100 0x0 0x0 0x0 0x0>;
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				device_type = "pci";
> +				ranges;
> +			};

[Severity: Critical]
Does the addition of this second child node trigger a buffer overflow
regression in the pwrctrl-tc9563 driver?

The tc9563_pwrctrl_probe() function loops over all children of the pcie@3,0
node and increments a shared port index for each child. Since the driver's
tc9563->cfg array is sized to 5, the addition of this node causes the port
index to reach 5.

This results in an out-of-bounds write when of_property_read_bool() writes to
cfg[5].disable_dfe, which can corrupt the tc9563->adapter pointer on 64-bit
platforms and cause a panic.

> +		};
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3