* [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev
@ 2026-09-19 22:26 Hui Peng
2026-09-21 5:27 ` [PATCH v2 1/2] PCI/VGA: fix inverted vga_tryget() return check in vga_arb_write() Hui Peng
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Hui Peng @ 2026-09-19 22:26 UTC (permalink / raw)
To: bhelgaas, airlied; +Cc: linux-pci, dri-devel, linux-kernel
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 <benquike@gmail.com>
---
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);
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/2] PCI/VGA: fix inverted vga_tryget() return check in vga_arb_write()
2026-09-19 22:26 [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Hui Peng
@ 2026-09-21 5:27 ` Hui Peng
2026-09-21 5:27 ` [PATCH v2 2/2] PCI/VGA: invalidate open user references in vga_arbiter_del_pci_device() Hui Peng
2026-09-21 10:39 ` [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Jani Nikula
2026-09-21 15:09 ` krzk
2 siblings, 1 reply; 5+ messages in thread
From: Hui Peng @ 2026-09-21 5:27 UTC (permalink / raw)
To: bhelgaas, airlied; +Cc: benh, linux-pci, linux-kernel, stable, benquike
In vga_arb_write(), the "trylock" command handler calls
vga_tryget(pdev, io_state) and checks:
if (vga_tryget(pdev, io_state)) {
/* Update the client's locks lists... */
However, vga_tryget() returns 0 on success and a negative errno (-EBUSY)
when the VGA resources are already locked. Because the check tests for a
non-zero return value, a successful vga_tryget() skips incrementing
priv->cards[i].io_cnt / mem_cnt and skips reporting success, leaking the
acquired VGA lock, whereas a failed vga_tryget() (-EBUSY) erroneously
records the lock in priv->cards[i] so a subsequent "unlock" or file
release decrements the lock count without holding the lock.
Check if (vga_tryget(pdev, io_state) == 0) in vga_arb_write().
Tested in QEMU against Linux 7.3.0-rc3 by opening /dev/vga_arbiter and
writing "trylock io+mem" followed by closing the file descriptor. On the
unfixed kernel, write("trylock io+mem") acquires the VGA lock
(locks=io+mem(1:1)) but takes the error branch and returns -EBUSY (-16)
without recording the lock in priv->cards[], permanently leaking the
lock on close(); with this patch applied, write("trylock io+mem")
succeeds (ret = 14) and close() cleanly releases the lock
(locks=none(0:0)).
Fixes: deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v2:
- Split the vga_tryget() return check fix in vga_arb_write() from the
PCI_INVALID_CARD hot-unplug cleanup fix into a 2-patch series, and fix
the commit message to reference vga_arb_write(), as noted by Sashiko.
- Update Fixes: tag from the file-move commit 1d38fe6ee6a8 to
deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux").
drivers/pci/vgaarb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 3de05aee7859..350ab9624eb9 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -1272,7 +1272,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf,
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) {
--
2.49.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 2/2] PCI/VGA: invalidate open user references in vga_arbiter_del_pci_device()
2026-09-21 5:27 ` [PATCH v2 1/2] PCI/VGA: fix inverted vga_tryget() return check in vga_arb_write() Hui Peng
@ 2026-09-21 5:27 ` Hui Peng
0 siblings, 0 replies; 5+ messages in thread
From: Hui Peng @ 2026-09-21 5:27 UTC (permalink / raw)
To: bhelgaas, airlied; +Cc: benh, linux-pci, linux-kernel, stable, benquike
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 <benquike@gmail.com>
---
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev
2026-09-19 22:26 [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Hui Peng
2026-09-21 5:27 ` [PATCH v2 1/2] PCI/VGA: fix inverted vga_tryget() return check in vga_arb_write() Hui Peng
@ 2026-09-21 10:39 ` Jani Nikula
2026-09-21 15:09 ` krzk
2 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2026-09-21 10:39 UTC (permalink / raw)
To: Hui Peng, bhelgaas, airlied; +Cc: linux-pci, dri-devel, linux-kernel
On Sat, 19 Sep 2026, Hui Peng <benquike@gmail.com> wrote:
> Fix two issues in drivers/pci/vgaarb.c:
One change per patch, please.
>
> 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 <benquike@gmail.com>
> ---
> 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);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev
2026-09-19 22:26 [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Hui Peng
2026-09-21 5:27 ` [PATCH v2 1/2] PCI/VGA: fix inverted vga_tryget() return check in vga_arb_write() Hui Peng
2026-09-21 10:39 ` [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Jani Nikula
@ 2026-09-21 15:09 ` krzk
2 siblings, 0 replies; 5+ messages in thread
From: krzk @ 2026-09-21 15:09 UTC (permalink / raw)
To: Hui Peng; +Cc: dri-devel, airlied, bhelgaas, linux-kernel, linux-pci
On Sat, 19 Sep 2026 22:26:29 +0000, Hui Peng wrote:
> 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 <benquike@gmail.com>
> ---
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem, how to document usage of LLM and how what you should not
do if this was posted in a good faith.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-21 15:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 22:26 [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Hui Peng
2026-09-21 5:27 ` [PATCH v2 1/2] PCI/VGA: fix inverted vga_tryget() return check in vga_arb_write() Hui Peng
2026-09-21 5:27 ` [PATCH v2 2/2] PCI/VGA: invalidate open user references in vga_arbiter_del_pci_device() Hui Peng
2026-09-21 10:39 ` [PATCH] PCI/VGA: fix inverted vga_tryget() return check and clear removed pci_dev Jani Nikula
2026-09-21 15:09 ` krzk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®