Fix two issues in drivers/pci/vgaarb.c:
1. In vga_arb_device_init() / vga_arbiter_From_pci_dev(), vga_tryget()
returns 0 on success and a negative errno on failure, so checking if
(!vga_tryget(...)) is inverted and skips unlocking on success.
2. In vga_arbiter_del_pci_device(), clear any open vga_arb_user_init
priv->target references pointing to the removed pci_dev so subsequent
writes or close() on /dev/vga_arbiter do not dereference a freed
pci_dev.
Fixes: 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to drivers/pci")
Assisted-by: LLM
Signed-off-by: Hui Peng <[email protected]>
---
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 3de05aee7859..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,12 +1282,12 @@ 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;
}
- if (vga_tryget(pdev, io_state)) {
+ if (vga_tryget(pdev, io_state) == 0) {
/* Update the client's locks lists... */
for (i = 0; i < MAX_USER_CARDS; i++) {
if (priv->cards[i].pdev == pdev) {
@@ -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);
