linux-kernel - Re: [PATCH] PCI: Fix endianness issues in pci_bus_read

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <74c17623-f1b5-4b28-a118-4e828e1e711a@app.fastmail.com>
Date: Mon, 04 Aug 2025 10:03:43 +0200
From: "Arnd Bergmann" <arnd@...nel.org>
To: "Hans Zhang" <18255117159@....com>, "Gerd Bayer" <gbayer@...ux.ibm.com>,
 "Manivannan Sadhasivam" <mani@...nel.org>,
 "Hans Zhang" <hans.zhang@...tech.com>
Cc: "Bjorn Helgaas" <helgaas@...nel.org>, bhelgaas@...gle.com,
 "Alexander Gordeev" <agordeev@...ux.ibm.com>,
 "Christian Borntraeger" <borntraeger@...ux.ibm.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 jingoohan1@...il.com,
 Krzysztof Wilczyński <kwilczynski@...nel.org>,
 linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
 linux-next <linux-next@...r.kernel.org>, linux-pci@...r.kernel.org,
 "Lorenzo Pieralisi" <lpieralisi@...nel.org>, "Rob Herring" <robh@...nel.org>,
 "Niklas Schnelle" <schnelle@...ux.ibm.com>,
 "Geert Uytterhoeven" <geert@...ux-m68k.org>
Subject: Re: [PATCH] PCI: Fix endianness issues in pci_bus_read_config()

On Mon, Aug 4, 2025, at 05:06, Hans Zhang wrote:
> On 2025/8/1 19:30, Gerd Bayer wrote:
>> On Fri, 2025-08-01 at 16:24 +0530, Manivannan Sadhasivam wrote:
>   }
>
> +#define CDNS_PCI_OP_READ(size, type, len)		\
> +static inline int cdns_pcie_read_cfg_##size		\
> +	(struct cdns_pcie *pcie, int where, type *val)	\
> +{							\
> +	if (len == 4)					\
> +		*val = cdns_pcie_readl(pcie, where);	\
> +	else if (len == 2)				\
> +		*val = cdns_pcie_readw(pcie, where);	\
> +	else if (len == 1)				\
> +		*val = cdns_pcie_readb(pcie, where);	\
> +	else						\
> +		return PCIBIOS_BAD_REGISTER_NUMBER;	\
> +							\
> +	return PCIBIOS_SUCCESSFUL;			\
> +}
> +
> +CDNS_PCI_OP_READ(byte, u8, 1)
> +CDNS_PCI_OP_READ(word, u16, 2)
> +CDNS_PCI_OP_READ(dword, u32, 4)
> +

This is fine for the calling conventions, but the use of a macro
doesn't really help readability, so I'd suggest just having
separate inline functions if they are even needed.

> @@ -112,17 +110,17 @@ int pci_bus_read_config(void *priv, unsigned int 
> devfn, int where, u32 size,
>   ({									\
>   	int __ttl = PCI_FIND_CAP_TTL;					\
>   	u8 __id, __found_pos = 0;					\
> -	u32 __pos = (start);						\
> -	u32 __ent;							\
> +	u8 __pos = (start);						\
> +	u16 __ent;							\
>   									\
> -	read_cfg(args, __pos, 1, &__pos);				\
> +	read_cfg##_byte(args, __pos, &__pos);				\
>   									\
>   	while (__ttl--) {						\
>   		if (__pos < PCI_STD_HEADER_SIZEOF)			\
>   			break;						\
>   									\
>   		__pos = ALIGN_DOWN(__pos, 4);				\
> -		read_cfg(args, __pos, 2, &__ent);			\
> +		read_cfg##_word(args, __pos, &__ent);			\
>   									\
>   		__id = FIELD_GET(PCI_CAP_ID_MASK, __ent);		\
>   		if (__id == 0xff)					\

I still don't feel great about this macro either, and suspect
we should have a better abstraction with fixed types and a
global function to do it, but I don't see anything obviously
wrong here either.

     Arnd