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: <9a155e45-f723-4eec-81d3-2547bfe9a4e9@cixtech.com>
Date: Fri, 1 Aug 2025 17:25:51 +0800
From: Hans Zhang <hans.zhang@...tech.com>
To: Manivannan Sadhasivam <mani@...nel.org>, 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>,
 ilpo.jarvinen@...ux.intel.com, geert@...ux-m68k.org
Subject: Re: [PATCH] PCI: Fix endianness issues in pci_bus_read_config()



On 2025/8/1 16:18, Manivannan Sadhasivam wrote:
> EXTERNAL EMAIL
> 
> 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.
> 

Dear all,

I am once again deeply sorry for the problems that occurred in this 
series. I only test pulling the ARM platform.

Thank you very much, Gerd, for reporting the problem.

Thank you all for your discussions and suggestions for revision.

Hi Mani,

Geert provided a solution. My patch based on this is as follows. Please 
check if there are any problems.
https://lore.kernel.org/linux-pci/CAMuHMdVwFeV46oCid_sMHjXfP+yyGTpBfs9t3uaa=wRxNcSOAQ@mail.gmail.com/

Also, please ask Gerd to help test whether it works properly. Thank you 
very much.


If there are no issues, am I sending the new version? Can this series of 
pacth 0001 be directly replaced?




The patch is as follows:
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index ba66f55d2524..2bfd8fc1c0f5 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -89,15 +89,25 @@ int pci_bus_read_config(void *priv, unsigned int 
devfn, int where, u32 size,
                         u32 *val)
  {
         struct pci_bus *bus = priv;
+       int rc;
+
+       if (size == 1) {
+               u8 byte;
+
+               rc = pci_bus_read_config_byte(bus, devfn, where, &byte);
+               *val = byte;
+       } else if (size == 2) {
+               u16 word;
+
+               rc = pci_bus_read_config_word(bus, devfn, where, &word);
+               *val = word;
+       } else if (size == 4) {
+               rc = pci_bus_read_config_dword(bus, devfn, where, val);
+       } else {
+               rc = PCIBIOS_BAD_REGISTER_NUMBER;
+       }

-       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;
+       return rc;
  }

  int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn,



Best regards,
Hans