linux-kernel - Re: [PATCH next] PCI: imx6: Add error handling in imx_pcie_cpu_addr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZtnMERH+/3HiqNtg@lizhi-Precision-Tower-5810>
Date: Thu, 5 Sep 2024 11:19:45 -0400
From: Frank Li <Frank.li@....com>
To: Riyan Dhiman <riyandhiman14@...il.com>
Cc: hongxing.zhu@....com, l.stach@...gutronix.de, lpieralisi@...nel.org,
	bhelgaas@...gle.com, linux-pci@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, imx@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH next] PCI: imx6: Add error handling in
 imx_pcie_cpu_addr_fixup()

On Thu, Sep 05, 2024 at 11:12:55AM +0530, Riyan Dhiman wrote:
> Added error handling to the imx_pcie_cpu_addr_fixup function for cases
> where the memory window retrieval fails. The initial implementation did
> not have error handling, and the function simply returned cpu_addr without
> checking for failure conditions.
>
> I have added -0ULL as a error return code but what should be the ideal return
> code for error handling in this function, given the u64 return type? Common
> approaches include returning ~0ULL or another invalid address value, but
> clarification on best practices would be appreciated.
>
> Signed-off-by: Riyan Dhiman <riyandhiman14@...il.com>
> ---

It is not necessary, because pp->bridge->windows should be always success.
If it is wrong, whole PCI will not work.

Even it is wrong, return 0 also wrong. dwc use it do out-bound ATU setting.
Map address 0 to outbound range have unexpected behaviors.

If it is scanned by static tool, it should be false alarm. If you really
met the issue, let me know. It is too late check it here.

Frank

> Compile tested only. I am new to the PCI subsystem and not sure how to test these
> modules. Do I need dedicated hardware, or is there another way to test these changes?
>
>  drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 0dbc333adcff..6af39852d2c2 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1023,6 +1023,10 @@ static u64 imx_pcie_cpu_addr_fixup(struct dw_pcie *pcie, u64 cpu_addr)
>  		return cpu_addr;
>
>  	entry = resource_list_first_type(&pp->bridge->windows, IORESOURCE_MEM);
> +	if(!entry) {
> +		dev_err(pcie->dev, "Unable to get memory window.");
> +		return -0ULL;
> +	}
>  	offset = entry->offset;
>
>  	return (cpu_addr - offset);
> --
> 2.46.0
>