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 for Android: free password hash cracker in your pocket

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

Message-ID: <vf65usnffqzlkgijm72nuaslxnflwrugc25vw6q6blbn2s2d2s@b35vjkowd6yc>
Date: Fri, 1 Aug 2025 13:48:09 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Arnd Bergmann <arnd@...nel.org>
Cc: Bjorn Helgaas <helgaas@...nel.org>, Gerd Bayer <gbayer@...ux.ibm.com>, 
	Hans Zhang <18255117159@....com>, 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>
Subject: Re: [PATCH] PCI: Fix endianness issues in pci_bus_read_config()

On Thu, Jul 31, 2025 at 09:01:17PM GMT, Arnd Bergmann wrote:
> On Thu, Jul 31, 2025, at 20:39, Bjorn Helgaas wrote:
> > On Thu, Jul 31, 2025 at 07:38:58PM +0200, Gerd Bayer wrote:
> >>  
> >> -	if (size == 1)
> >> -		return pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
> >> -	else if (size == 2)
> >> -		return pci_bus_read_config_word(bus, devfn, where, (u16 *)val);
> >> -	else if (size == 4)
> >> -		return pci_bus_read_config_dword(bus, devfn, where, val);
> >> -	else
> >> -		return PCIBIOS_BAD_REGISTER_NUMBER;
> >> +	if (size == 1) {
> >> +		rc = pci_bus_read_config_byte(bus, devfn, where, (u8 *)val);
> >> +#if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
> >> +		*val = ((*val >> 24) & 0xff);
> >> +#endif
> >
> > Yeah, this is all pretty ugly.  Obviously the previous code in
> > __pci_find_next_cap_ttl() didn't need this.  My guess is that was
> > because the destination for the read data was always the correct type
> > (u8/u16/u32), but here we always use a u32 and cast it to the
> > appropriate type.  Maybe we can use the correct types here instead of
> > the casts?
> 
> Agreed, the casts here just add more potential for bugs.
> 

Ack. Missed the obvious issue during review.

> The pci_bus_read_config() interface itself may have been a
> mistake, can't the callers just use the underlying helpers
> directly?
> 

They can! Since the callers of this API is mostly the macros, we can easily
implement the logic to call relevant accessors based on the requested size.

Hans, could you please respin the series based the feedback since the series is
dropped for 6.17.

- Mani

-- 
மணிவண்ணன் சதாசிவம்