Linux PCI subsystem development
 help / color / mirror / Atom feed
* [BUG] PCI/VGA: KASAN slab-use-after-free in dev_driver_string from vga_arb_release
@ 2026-06-22  4:01 Shuangpeng
  2026-06-30 16:30 ` [PATCH] PCI/VGA: vgaarb: Hold pci_dev references for per-file cards Shuangpeng Bai
  0 siblings, 1 reply; 3+ messages in thread
From: Shuangpeng @ 2026-06-22  4:01 UTC (permalink / raw)
  To: bhelgaas; +Cc: linux-pci, linux-kernel

Hi Kernel Maintainers,

I hit the following report while testing current upstream kernel:

KASAN: slab-use-after-free in dev_driver_string from vga_arb_release

on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)

The reproducer and .config files are here.
https://gist.github.com/shuangpengbai/47ec0715a0120beaacf5009460b90687

I'm happy to test debug patches or provide additional information.

Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>

[  250.268327][ T8342] ==================================================================
[  250.271034][ T8342] BUG: KASAN: slab-use-after-free in dev_driver_string (drivers/base/core.c:2406)
[  250.273371][ T8342] Read of size 8 at addr ffff88810aebc138 by task bash/8342
[  250.274604][ T8342]
[  250.275019][ T8342] CPU: 0 UID: 0 PID: 8342 Comm: bash Not tainted 7.1.0-rc5+ #106 PREEMPT(full)
[  250.275027][ T8342] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[  250.275031][ T8342] Call Trace:
[  250.275044][ T8342]  <TASK>
[  250.275049][ T8342]  dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
[  250.275108][ T8342]  print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
[  250.275177][ T8342]  kasan_report (mm/kasan/report.c:595)
[  250.275190][ T8342]  dev_driver_string (drivers/base/core.c:2406)
[  250.275196][ T8342]  __dynamic_dev_dbg (lib/dynamic_debug.c:906)
[  250.275228][ T8342]  vga_arb_release (drivers/pci/vgaarb.c:1436)
[  250.275245][ T8342]  __fput (fs/file_table.c:510)
[  250.275260][ T8342]  fput_close_sync (fs/file_table.c:615)
[  250.275265][ T8342]  __x64_sys_close (fs/open.c:1507 fs/open.c:1492 fs/open.c:1492)
[  250.275271][ T8342]  do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
[  250.275327][ T8342]  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
[  250.275333][ T8342] RIP: 0033:0x7f65140dbc03
[  250.275341][ T8342] Code: e9 37 ff ff ff e8 2d f9 01 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 03 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 45 c3 0f 1f 40 00 48 83 ec 18 89 7c 24 0c e8
[  250.275346][ T8342] RSP: 002b:00007ffce08e3808 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
[  250.275363][ T8342] RAX: ffffffffffffffda RBX: 0000564bd1808ba0 RCX: 00007f65140dbc03
[  250.275367][ T8342] RDX: 00000000ffffffff RSI: 0000000000000001 RDI: 000000000000000a
[  250.275371][ T8342] RBP: 00007ffce08e3890 R08: 0000000000000000 R09: 0000000000000000
[  250.275374][ T8342] R10: 00007f651417efc0 R11: 0000000000000246 R12: 0000000000000009
[  250.275377][ T8342] R13: 0000000000000009 R14: 000000000000000a R15: 0000000000000000
[  250.275393][ T8342]  </TASK>
[  250.275395][ T8342]
[  250.295059][ T8342] Freed by task 8342 on cpu 1 at 249.233016s:
[  250.295689][ T8342]  kasan_save_track (mm/kasan/common.c:57 mm/kasan/common.c:78)
[  250.296168][ T8342]  kasan_save_free_info (mm/kasan/generic.c:584)
[  250.296682][ T8342]  __kasan_slab_free (mm/kasan/common.c:253 mm/kasan/common.c:285)
[  250.297176][ T8342]  kfree (include/linux/kasan.h:235 mm/slub.c:2689 mm/slub.c:6251 mm/slub.c:6566)
[  250.297593][ T8342]  device_release (drivers/base/core.c:2570)
[  250.298062][ T8342]  kobject_put (lib/kobject.c:689 lib/kobject.c:720 include/linux/kref.h:65 lib/kobject.c:737)
[  250.298528][ T8342]  pci_stop_and_remove_bus_device_locked (drivers/pci/remove.c:132 drivers/pci/remove.c:139)
[  250.299187][ T8342]  remove_store (drivers/pci/pci-sysfs.c:525)
[  250.299631][ T8342]  kernfs_fop_write_iter (fs/kernfs/file.c:352)
[  250.300179][ T8342]  vfs_write (fs/read_write.c:595 fs/read_write.c:688)
[  250.300622][ T8342]  ksys_write (fs/read_write.c:740)
[  250.301054][ T8342]  do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
[  250.301525][ T8342]  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
[  250.302121][ T8342]
[  250.302372][ T8342] The buggy address belongs to the object at ffff88810aebc000
[  250.302372][ T8342]  which belongs to the cache kmalloc-4k of size 4096
[  250.303772][ T8342] The buggy address is located 312 bytes inside of
[  250.303772][ T8342]  freed 4096-byte region [ffff88810aebc000, ffff88810aebd000)
[  250.305161][ T8342]

Best,
Shuangpeng


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] PCI/VGA: vgaarb: Hold pci_dev references for per-file cards
  2026-06-22  4:01 [BUG] PCI/VGA: KASAN slab-use-after-free in dev_driver_string from vga_arb_release Shuangpeng
@ 2026-06-30 16:30 ` Shuangpeng Bai
  2026-06-30 16:46   ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Shuangpeng Bai @ 2026-06-30 16:30 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel, Shuangpeng Bai

The VGA arbiter stores pci_dev pointers in each /dev/vga_arbiter file's
private state to track the target device and per-card lock counts.  These
pointers may outlive PCI hot-remove, but the private state did not hold
pci_dev references for them.

After a userspace client opens /dev/vga_arbiter, selects and locks a PCI
VGA device, removes that device via sysfs, and closes the old fd,
vga_arb_release() can still dereference uc->pdev for dev_dbg() and
vga_put().  If the PCI device has already been freed, dynamic debug may
read the freed struct device in dev_driver_string(), triggering a KASAN
use-after-free.

Make each non-NULL priv->cards[] entry own one pci_dev reference.
priv->target is kept only as an alias of a tracked cards[] entry and does
not own an extra reference.  When selecting a target, consume the lookup
reference by either storing it in a new cards[] entry or dropping it if
the device is already tracked.  On release, use the still-referenced
pdevs to clean up lock accounting, then drop the pci_dev references after
leaving the spinlock.

Take vga_lock while converting vga_default_device() into a referenced
pci_dev and while looking up vgadev entries, so the reference acquisition
and vga_list traversal are serialized with VGA device removal.

Fixes: deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux")
Closes: https://lore.kernel.org/r/330FD8DD-ECBF-4531-900E-5B976FA9DF90@gmail.com/
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
---
 drivers/pci/vgaarb.c | 87 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c360eee11dd9..244671d14e08 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -1044,6 +1044,58 @@ struct vga_arb_private {
 	spinlock_t lock;
 };
 
+/*
+ * Each non-NULL priv->cards[i].pdev owns one pci_dev reference.
+ * priv->target is only an alias of one of priv->cards[] and does not
+ * own an extra reference.
+ *
+ * On success, this consumes the caller's @pdev reference.  If the device
+ * is already tracked, the temporary reference is dropped; otherwise it is
+ * stored in a new cards[] entry.  On failure, the caller still owns @pdev.
+ */
+static int vga_arb_set_target(struct vga_arb_private *priv,
+			      struct pci_dev *pdev)
+{
+	unsigned long flags;
+	int i, ret = -ENOMEM;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	for (i = 0; i < MAX_USER_CARDS; i++) {
+		if (priv->cards[i].pdev == pdev) {
+			priv->target = priv->cards[i].pdev;
+			ret = 0;
+			break;
+		}
+		if (!priv->cards[i].pdev) {
+			priv->cards[i].pdev = pdev;
+			priv->cards[i].io_cnt = 0;
+			priv->cards[i].mem_cnt = 0;
+			priv->target = priv->cards[i].pdev;
+			pdev = NULL;
+			ret = 0;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	if (!ret)
+		pci_dev_put(pdev);
+
+	return ret;
+}
+
+static struct pci_dev *vga_arb_get_default_pdev(void)
+{
+	struct pci_dev *pdev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vga_lock, flags);
+	pdev = pci_dev_get(vga_default_device());
+	spin_unlock_irqrestore(&vga_lock, flags);
+
+	return pdev;
+}
+
 static LIST_HEAD(vga_user_list);
 static DEFINE_SPINLOCK(vga_user_lock);
 
@@ -1294,13 +1346,14 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf,
 	} else if (strncmp(curr_pos, "target ", 7) == 0) {
 		unsigned int domain, bus, devfn;
 		struct vga_device *vgadev;
+		unsigned long flags;
 
 		curr_pos += 7;
 		remaining -= 7;
 		pr_debug("client 0x%p called 'target'\n", priv);
 		/* If target is default */
 		if (!strncmp(curr_pos, "default", 7))
-			pdev = pci_dev_get(vga_default_device());
+			pdev = vga_arb_get_default_pdev();
 		else {
 			if (!vga_pci_str_to_vars(curr_pos, remaining,
 						 &domain, &bus, &devfn)) {
@@ -1321,7 +1374,9 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf,
 				pdev);
 		}
 
+		spin_lock_irqsave(&vga_lock, flags);
 		vgadev = vgadev_find(pdev);
+		spin_unlock_irqrestore(&vga_lock, flags);
 		pr_debug("vgadev %p\n", vgadev);
 		if (vgadev == NULL) {
 			if (pdev) {
@@ -1333,18 +1388,8 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf,
 			goto done;
 		}
 
-		priv->target = pdev;
-		for (i = 0; i < MAX_USER_CARDS; i++) {
-			if (priv->cards[i].pdev == pdev)
-				break;
-			if (priv->cards[i].pdev == NULL) {
-				priv->cards[i].pdev = pdev;
-				priv->cards[i].io_cnt = 0;
-				priv->cards[i].mem_cnt = 0;
-				break;
-			}
-		}
-		if (i == MAX_USER_CARDS) {
+		ret_val = vga_arb_set_target(priv, pdev);
+		if (ret_val) {
 			vgaarb_dbg(&pdev->dev, "maximum user cards (%d) number reached, ignoring this one!\n",
 				MAX_USER_CARDS);
 			pci_dev_put(pdev);
@@ -1354,7 +1399,6 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf,
 		}
 
 		ret_val = count;
-		pci_dev_put(pdev);
 		goto done;
 
 
@@ -1394,6 +1438,7 @@ static __poll_t vga_arb_fpoll(struct file *file, poll_table *wait)
 
 static int vga_arb_open(struct inode *inode, struct file *file)
 {
+	struct pci_dev *pdev;
 	struct vga_arb_private *priv;
 	unsigned long flags;
 
@@ -1410,8 +1455,9 @@ static int vga_arb_open(struct inode *inode, struct file *file)
 	spin_unlock_irqrestore(&vga_user_lock, flags);
 
 	/* Set the client's lists of locks */
-	priv->target = vga_default_device(); /* Maybe this is still null! */
-	priv->cards[0].pdev = priv->target;
+	pdev = vga_arb_get_default_pdev(); /* Maybe this is still null! */
+	priv->cards[0].pdev = pdev;
+	priv->target = pdev;
 	priv->cards[0].io_cnt = 0;
 	priv->cards[0].mem_cnt = 0;
 
@@ -1422,8 +1468,9 @@ static int vga_arb_release(struct inode *inode, struct file *file)
 {
 	struct vga_arb_private *priv = file->private_data;
 	struct vga_arb_user_card *uc;
+	struct pci_dev *pdevs[MAX_USER_CARDS];
 	unsigned long flags;
-	int i;
+	int i, nr_pdevs = 0;
 
 	pr_debug("%s\n", __func__);
 
@@ -1439,9 +1486,15 @@ static int vga_arb_release(struct inode *inode, struct file *file)
 			vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);
 		while (uc->mem_cnt--)
 			vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
+		pdevs[nr_pdevs++] = uc->pdev;
+		uc->pdev = NULL;
 	}
+	priv->target = NULL;
 	spin_unlock_irqrestore(&vga_user_lock, flags);
 
+	for (i = 0; i < nr_pdevs; i++)
+		pci_dev_put(pdevs[i]);
+
 	kfree(priv);
 
 	return 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] PCI/VGA: vgaarb: Hold pci_dev references for per-file cards
  2026-06-30 16:30 ` [PATCH] PCI/VGA: vgaarb: Hold pci_dev references for per-file cards Shuangpeng Bai
@ 2026-06-30 16:46   ` sashiko-bot
  0 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-06-30 16:46 UTC (permalink / raw)
  To: Shuangpeng Bai; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent `lock`, `unlock`, and `trylock` commands in `vga_arb_write` cause data races on `uc->io_cnt` and `uc->mem_cnt`, leading to global resource leaks or severe CPU stalls (Denial of Service).
--

commit 17b88a219897b83c9eda1acce8ef44e9f9f96033
Author: Shuangpeng Bai <shuangpeng.kernel@gmail.com>

PCI/VGA: vgaarb: Hold pci_dev references for per-file cards
    
The VGA arbiter stores pci_dev pointers in each /dev/vga_arbiter file's
private state to track the target device and per-card lock counts. This
patch ensures each tracked non-NULL cards[] entry owns a pci_dev reference
to prevent KASAN use-after-free bugs if the device is removed via sysfs.

Closes: https://lore.kernel.org/r/330FD8DD-ECBF-4531-900E-5B976FA9DF90@gmail.com/

> diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
> --- a/drivers/pci/vgaarb.c
> +++ b/drivers/pci/vgaarb.c
> @@ -1422,8 +1468,9 @@ static int vga_arb_release(struct inode *inode, struct file *file)
[ ... ]
> @@ -1439,9 +1486,15 @@ static int vga_arb_release(struct inode *inode, struct file *file)
>  		if (uc->pdev == NULL)
>  			continue;
>  		vgaarb_dbg(&uc->pdev->dev, "uc->io_cnt == %d, uc->mem_cnt == %d\n",
>  			uc->io_cnt, uc->mem_cnt);
>  		while (uc->io_cnt--)
>  			vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);

[Severity: High]
This is a pre-existing issue, but can concurrent calls to vga_arb_write()
with unlock commands cause a denial of service here?

If userspace uses pwrite64 or io_uring to issue concurrent unlock commands
on the same file descriptor, two threads can simultaneously pass the
uc->io_cnt != 0 validation check in vga_arb_write().

Because the decrement happens locklessly after a blocking call to vga_put():

vga_arb_write() {
        if (io_state & VGA_RSRC_LEGACY_IO && uc->io_cnt == 0) {
                ret_val = -EINVAL;
                goto done;
        }
        ...
        vga_put(pdev, io_state);
        
        if (io_state & VGA_RSRC_LEGACY_IO)
                uc->io_cnt--;
}

Both threads can execute uc->io_cnt--, which underflows the counter to
UINT_MAX.

When the file is closed, vga_arb_release() executes this loop:

while (uc->io_cnt--)
        vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);

This loop executes ~4.29 billion times while holding the vga_user_lock
spinlock with interrupts disabled, completely stalling the CPU.

Does this resource counting need to be properly serialized or converted to
atomic types to prevent a hard lockup?

>  		while (uc->mem_cnt--)
>  			vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
> +		pdevs[nr_pdevs++] = uc->pdev;
> +		uc->pdev = NULL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/86061f17ea5f1fb76791ca5cc787ce84b6752f46.1782788237.git.shuangpeng.kernel@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-30 16:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22  4:01 [BUG] PCI/VGA: KASAN slab-use-after-free in dev_driver_string from vga_arb_release Shuangpeng
2026-06-30 16:30 ` [PATCH] PCI/VGA: vgaarb: Hold pci_dev references for per-file cards Shuangpeng Bai
2026-06-30 16:46   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox