[PATCH v2 01/15] pciutils-lspci: Fix unsynchronized caches in lspci struct device and pci struct pci_dev

From: Nikita Proshkin <n.proshkin@yadro.com>
To: <linux-pci@vger.kernel.org>, Martin Mares <mj@ucw.cz>
Cc: <linux@yadro.com>, Bjorn Helgaas <helgaas@kernel.org>,
	Sergei Miroshnichenko <s.miroshnichenko@yadro.com>,
	Nikita Proshkin <n.proshkin@yadro.com>
Subject: [PATCH v2 01/15] pciutils-lspci: Fix unsynchronized caches in lspci struct device and pci struct pci_dev
Date: Wed, 27 Dec 2023 14:44:50 +0500	[thread overview]
Message-ID: <20231227094504.32257-2-n.proshkin@yadro.com> (raw)
In-Reply-To: <20231227094504.32257-1-n.proshkin@yadro.com>

lspci initializes both caches for the device to the same memory block in
its scan_device function. Latter calls to config_fetch function will
realloc cache in struct device, but not in struct pci_dev leading to
the invalid pointer in the latter. pci_dev cache is used by pci_read_*
functions, what will lead to a possible use-after-free situations.

Example:

With patch:

diff --git a/ls-caps.c b/ls-caps.c
index a481b16..b454843 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -1802,6 +1802,7 @@ show_caps(struct device *d, int where)
 	      break;
 	    case PCI_CAP_ID_EXP:
 	      type = cap_express(d, where, cap);
+        struct pci_cap* test = pci_find_cap(d->dev, PCI_CAP_ID_EXP, PCI_CAP_NORMAL);
 	      can_have_ext_caps = 1;
 	      break;
 	    case PCI_CAP_ID_MSIX:

valgrind run:
valgrind ./lspci -vvvs 7:0.0

...
==22835== Invalid read of size 2
==22835==    at 0x11A90A: pci_read_word (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x11EBEC: pci_scan_caps (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x11AC00: pci_fill_info_v38 (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x11ED73: pci_find_cap (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x1126FA: show_caps (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10E860: show_device (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10BFA3: main (in /home/merlin/git/pciutils/lspci)
==22835==  Address 0x5249276 is 6 bytes inside a block of size 64 free'd
==22835==    at 0x4E0A13B: realloc (vg_replace_malloc.c:1649)
==22835==    by 0x119BCC: xrealloc (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10CD2C: config_fetch (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x110DAA: show_caps (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10E860: show_device (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10BFA3: main (in /home/merlin/git/pciutils/lspci)
==22835==  Block was alloc'd at
==22835==    at 0x4E050B5: malloc (vg_replace_malloc.c:431)
==22835==    by 0x119B9C: xmalloc (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10CE80: scan_device (in /home/merlin/git/pciutils/lspci)
==22835==    by 0x10BF0F: main (in /home/merlin/git/pciutils/lspci)
...

Reviewed-by: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
Signed-off-by: Nikita Proshkin <n.proshkin@yadro.com>
---
 lspci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lspci.c b/lspci.c
index 9452cd3..071cc11 100644
--- a/lspci.c
+++ b/lspci.c
@@ -107,6 +107,7 @@ config_fetch(struct device *d, unsigned int pos, unsigned int len)
       d->config = xrealloc(d->config, d->config_bufsize);
       d->present = xrealloc(d->present, d->config_bufsize);
       memset(d->present + orig_size, 0, d->config_bufsize - orig_size);
+      pci_setup_cache(d->dev, d->config, d->dev->cache_len);
     }
   result = pci_read_block(d->dev, pos, d->config + pos, len);
   if (result)
-- 
2.34.1

