Re: [PATCH 1/2] Revert "PCI/VPD: Allow access to valid parts of VPD if some is invalid"

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Hannes, who did a lot of related VPD work and reviewed the
original posting at
https://lore.kernel.org/r/20210715215959.2014576-6-helgaas@kernel.org/]

On Thu, Mar 07, 2024 at 05:09:27PM +0100, Josselin Mouette wrote:
> When a device returns invalid VPD data, it can be misused by other
> code paths in kernel space or user space, and there are instances
> in which this seems to cause memory corruption.

More of the background from Josselin at
https://lore.kernel.org/r/aaea0b30c35bb73b947727e4b3ec354d6b5c399c.camel@exaion.com

This is a regression, and obviously needs to be fixed somehow, but I'm
a bit hesitant to revert this until we understand the problem better.
If there's a memory corruption lurking and a revert hides the
corruption so we never fix it, I'm not sure that's an improvement
overall.

> There is no sensible reason why the kernel would provide userspace
> or drivers with invalid and potentially dangerous data.
> 
> This reverts commit 5fe204eab174fd474227f23fd47faee4e7a6c000.
> ---
>  drivers/pci/vpd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c
> index 485a642b9304..daaa208c9d9c 100644
> --- a/drivers/pci/vpd.c
> +++ b/drivers/pci/vpd.c
> @@ -68,7 +68,7 @@ static size_t pci_vpd_size(struct pci_dev *dev)
>                         if (pci_read_vpd_any(dev, off + 1, 2,
> &header[1]) != 2) {
>                                 pci_warn(dev, "failed VPD read at
> offset %zu\n",
>                                          off + 1);
> -                               return off ?: PCI_VPD_SZ_INVALID;
> +                               return PCI_VPD_SZ_INVALID;
>                         }
>                         size = pci_vpd_lrdt_size(header);
>                         if (off + size > PCI_VPD_MAX_SIZE)
> @@ -87,13 +87,13 @@ static size_t pci_vpd_size(struct pci_dev *dev)
>                                 return off;
>                 }
>         }
> -       return off;
> +       return PCI_VPD_SZ_INVALID;
>  
>  error:
>         pci_info(dev, "invalid VPD tag %#04x (size %zu) at offset
> %zu%s\n",
>                  header[0], size, off, off == 0 ?
>                  "; assume missing optional EEPROM" : "");
> -       return off ?: PCI_VPD_SZ_INVALID;
> +       return PCI_VPD_SZ_INVALID;
>  }
>  
>  static bool pci_vpd_available(struct pci_dev *dev, bool check_size)
> -- 
> 2.39.2
>