When a PCI VGA device is removed while /dev/vga_arbiter is open, vga_arbiter_del_pci_device() removes and frees the vgadev entry, but any open struct vga_arb_private on vga_user_list retains dangling pointers to the removed pdev in priv->target and priv->cards[i].pdev. Subsequent writes to /dev/vga_arbiter or closing the file descriptor in vga_arb_release() dereference the removed pdev. Mark matching priv->target and priv->cards[i].pdev entries as PCI_INVALID_CARD under vga_user_lock in vga_arbiter_del_pci_device(), and treat PCI_INVALID_CARD like NULL in vga_arb_write() and vga_arb_release(). Tested in QEMU against Linux 7.3.0-rc3 by opening /dev/vga_arbiter, selecting PCI:0000:00:02.0, hot-removing the PCI VGA device via /sys/bus/pci/devices/0000:00:02.0/remove, and writing "unlock io+mem". On the unfixed kernel, priv->target retains a dangling pointer to the removed pci_dev so write("unlock io+mem") proceeds past the NULL check and returns -EINVAL (-22); with this patch applied, priv->target is set to PCI_INVALID_CARD on removal and write("unlock io+mem") immediately returns -ENODEV (-19). Fixes: deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Hui Peng --- Changes in v2: - Split out from the vga_tryget() return check fix into patch 2/2. - Update Fixes: tag from the file-move commit 1d38fe6ee6a8 to deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux"). drivers/pci/vgaarb.c | 71 +++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c index 350ab9624eb9..4f13e542505a 100644 --- a/drivers/pci/vgaarb.c +++ b/drivers/pci/vgaarb.c @@ -818,11 +818,33 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) return false; } +#define MAX_USER_CARDS CONFIG_VGA_ARB_MAX_GPUS +#define PCI_INVALID_CARD ((struct pci_dev *)-1UL) + +/* Each user has an array of these, tracking which cards have locks */ +struct vga_arb_user_card { + struct pci_dev *pdev; + unsigned int mem_cnt; + unsigned int io_cnt; +}; + +struct vga_arb_private { + struct list_head list; + struct pci_dev *target; + struct vga_arb_user_card cards[MAX_USER_CARDS]; + spinlock_t lock; +}; + +static LIST_HEAD(vga_user_list); +static DEFINE_SPINLOCK(vga_user_lock); + static bool vga_arbiter_del_pci_device(struct pci_dev *pdev) { struct vga_device *vgadev; + struct vga_arb_private *priv; unsigned long flags; bool ret = true; + int i; spin_lock_irqsave(&vga_lock, flags); vgadev = vgadev_find(pdev); @@ -845,6 +867,21 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev) wake_up_all(&vga_wait_queue); bail: spin_unlock_irqrestore(&vga_lock, flags); + if (ret) { + spin_lock_irqsave(&vga_user_lock, flags); + list_for_each_entry(priv, &vga_user_list, list) { + if (priv->target == pdev) + priv->target = PCI_INVALID_CARD; + for (i = 0; i < MAX_USER_CARDS; i++) { + if (priv->cards[i].pdev == pdev) { + priv->cards[i].pdev = PCI_INVALID_CARD; + priv->cards[i].io_cnt = 0; + priv->cards[i].mem_cnt = 0; + } + } + } + spin_unlock_irqrestore(&vga_user_lock, flags); + } kfree(vgadev); return ret; } @@ -1025,27 +1062,6 @@ EXPORT_SYMBOL(vga_client_register); * the arbiter. */ -#define MAX_USER_CARDS CONFIG_VGA_ARB_MAX_GPUS -#define PCI_INVALID_CARD ((struct pci_dev *)-1UL) - -/* Each user has an array of these, tracking which cards have locks */ -struct vga_arb_user_card { - struct pci_dev *pdev; - unsigned int mem_cnt; - unsigned int io_cnt; -}; - -struct vga_arb_private { - struct list_head list; - struct pci_dev *target; - struct vga_arb_user_card cards[MAX_USER_CARDS]; - spinlock_t lock; -}; - -static LIST_HEAD(vga_user_list); -static DEFINE_SPINLOCK(vga_user_lock); - - /* * Take a string in the format: "PCI:domain:bus:dev.fn" and return the * respective values. If the string is not in this format, return 0. @@ -1168,7 +1184,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, } pdev = priv->target; - if (priv->target == NULL) { + if (priv->target == NULL || priv->target == PCI_INVALID_CARD) { ret_val = -ENODEV; goto done; } @@ -1215,7 +1231,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, } pdev = priv->target; - if (priv->target == NULL) { + if (priv->target == NULL || priv->target == PCI_INVALID_CARD) { ret_val = -ENODEV; goto done; } @@ -1266,7 +1282,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, */ pdev = priv->target; - if (priv->target == NULL) { + if (priv->target == NULL || priv->target == PCI_INVALID_CARD) { ret_val = -ENODEV; goto done; } @@ -1335,7 +1351,8 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, for (i = 0; i < MAX_USER_CARDS; i++) { if (priv->cards[i].pdev == pdev) break; - if (priv->cards[i].pdev == NULL) { + if (priv->cards[i].pdev == NULL || + priv->cards[i].pdev == PCI_INVALID_CARD) { priv->cards[i].pdev = pdev; priv->cards[i].io_cnt = 0; priv->cards[i].mem_cnt = 0; @@ -1366,7 +1383,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, goto done; } pdev = priv->target; - if (priv->target == NULL) { + if (priv->target == NULL || priv->target == PCI_INVALID_CARD) { ret_val = -ENODEV; goto done; } @@ -1429,7 +1446,7 @@ static int vga_arb_release(struct inode *inode, struct file *file) list_del(&priv->list); for (i = 0; i < MAX_USER_CARDS; i++) { uc = &priv->cards[i]; - if (uc->pdev == NULL) + if (uc->pdev == NULL || uc->pdev == PCI_INVALID_CARD) continue; vgaarb_dbg(&uc->pdev->dev, "uc->io_cnt == %d, uc->mem_cnt == %d\n", uc->io_cnt, uc->mem_cnt); -- 2.49.0