On Sat, 19 Sep 2026, Hui Peng <[email protected]> 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 <[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);
--
Jani Nikula, Intel
