Re: [PATCH] Avoid an infinite loop in pci_get_rom_size()

[Bjorn Helgaas]

[+cc Michel]

On Sat, Jan 17, 2015 at 7:50 AM, Federico <federicotg@xxxxxxxxx> wrote:
> There's a soft_lockup triggered on boot by the radeon driver when it calls
> pci_get_rom_size() on a certain AMD Radeon R7 250X PCIe discrete graphics
> card.
>
> Kernel does not boot as it loops forever unless nomodeset is used, but then
> radeon driver is not usable as it requires KMS.
>
> If we have a PCIR structure with a zero size length, the loop iterating
> through those structure does not advance. It simply does "image += readw(pds
> + 16) * 512;", but if that field is zero we're back analyzing the same
> structure on the next loop.
>
> This is a panic stack trace generated using softlockup_panic=1
> https://launchpadlibrarian.net/194940395/20150114_205715.png
>
> There are other panic screen shots and log files in this bug report
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1386973
>
> The issue was discussed here
> http://lists.freedesktop.org/archives/dri-devel/2015-January/thread.html#75204
>
> And the solution was provided here:
> http://lists.freedesktop.org/archives/dri-devel/2015-January/075488.html
>
> This patch simple breaks the loop if the PCIR structure has zero length.
>
> As you can read in the mailing list discussion, I reported the bug, provided
> the screen shots, found the problem description online and tested the patch.
> The actual patch was provided by Michel DÃnzer from the dri-devel mailing
> list.

Thanks for the fix.  It needs to be posted with a Signed-off-by
attribution before I can apply it; see
https://www.kernel.org/doc/Documentation/SubmittingPatches for
details.  It sounds like this should come from Michel.

Bjorn

> diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c
> index f955edb..2fb5013 100644
> --- a/drivers/pci/rom.c
> +++ b/drivers/pci/rom.c
> @@ -75,6 +75,7 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem
> *rom, size_t size)
>      image = rom;
>      do {
>          void __iomem *pds;
> +        u16 length;
>          /* Standard PCI ROMs start out with these bytes 55 AA */
>          if (readb(image) != 0x55) {
>              dev_err(&pdev->dev, "Invalid ROM contents\n");
> @@ -93,8 +94,10 @@ size_t pci_get_rom_size(struct pci_dev *pdev, void
> __iomem *rom, size_t size)
>          if (readb(pds + 3) != 'R')
>              break;
>          last_image = readb(pds + 21) & 0x80;
> -        /* this length is reliable */
> -        image += readw(pds + 16) * 512;
> +        length = readw(pds + 16);
> +        if (!length)
> +            break;
> +        image += length * 512;
>      } while (!last_image);
>
>      /* never return a size larger than the PCI resource window */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/