4.20-stable review patch. If anyone has any objections, please let me know.
------------------
From: Marc Zyngier marc.zyngier@arm.com
commit 9791ec7df0e7b4d80706ccea8f24b6542f6059e9 upstream.
On systems or VMs where multiple devices share a single DevID (because they sit behind a PCI bridge, or because the HW is broken in funky ways), we reuse the save its_device structure in order to reflect this.
It turns out that there is a distinct lack of locking when looking up the its_device, and two device being probed concurrently can result in double allocations. That's obviously not nice.
A solution for this is to have a per-ITS mutex that serializes device allocation.
A similar issue exists on the freeing side, which can run concurrently with the allocation. On top of now taking the appropriate lock, we also make sure that a shared device is never freed, as we have no way to currently track the life cycle of such object.
Reported-by: Zheng Xiang zhengxiang9@huawei.com Tested-by: Zheng Xiang zhengxiang9@huawei.com Cc: stable@vger.kernel.org Signed-off-by: Marc Zyngier marc.zyngier@arm.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/irqchip/irq-gic-v3-its.c | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-)
--- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -97,9 +97,14 @@ struct its_device; * The ITS structure - contains most of the infrastructure, with the * top-level MSI domain, the command queue, the collections, and the * list of devices writing to it. + * + * dev_alloc_lock has to be taken for device allocations, while the + * spinlock must be taken to parse data structures such as the device + * list. */ struct its_node { raw_spinlock_t lock; + struct mutex dev_alloc_lock; struct list_head entry; void __iomem *base; phys_addr_t phys_base; @@ -156,6 +161,7 @@ struct its_device { void *itt; u32 nr_ites; u32 device_id; + bool shared; };
static struct { @@ -2422,6 +2428,7 @@ static int its_msi_prepare(struct irq_do struct its_device *its_dev; struct msi_domain_info *msi_info; u32 dev_id; + int err = 0;
/* * We ignore "dev" entierely, and rely on the dev_id that has @@ -2444,6 +2451,7 @@ static int its_msi_prepare(struct irq_do return -EINVAL; }
+ mutex_lock(&its->dev_alloc_lock); its_dev = its_find_device(its, dev_id); if (its_dev) { /* @@ -2451,18 +2459,22 @@ static int its_msi_prepare(struct irq_do * another alias (PCI bridge of some sort). No need to * create the device. */ + its_dev->shared = true; pr_debug("Reusing ITT for devID %x\n", dev_id); goto out; }
its_dev = its_create_device(its, dev_id, nvec, true); - if (!its_dev) - return -ENOMEM; + if (!its_dev) { + err = -ENOMEM; + goto out; + }
pr_debug("ITT %d entries, %d bits\n", nvec, ilog2(nvec)); out: + mutex_unlock(&its->dev_alloc_lock); info->scratchpad[0].ptr = its_dev; - return 0; + return err; }
static struct msi_domain_ops its_msi_domain_ops = { @@ -2566,6 +2578,7 @@ static void its_irq_domain_free(struct i { struct irq_data *d = irq_domain_get_irq_data(domain, virq); struct its_device *its_dev = irq_data_get_irq_chip_data(d); + struct its_node *its = its_dev->its; int i;
for (i = 0; i < nr_irqs; i++) { @@ -2580,8 +2593,14 @@ static void its_irq_domain_free(struct i irq_domain_reset_irq_data(data); }
- /* If all interrupts have been freed, start mopping the floor */ - if (bitmap_empty(its_dev->event_map.lpi_map, + mutex_lock(&its->dev_alloc_lock); + + /* + * If all interrupts have been freed, start mopping the + * floor. This is conditionned on the device not being shared. + */ + if (!its_dev->shared && + bitmap_empty(its_dev->event_map.lpi_map, its_dev->event_map.nr_lpis)) { its_lpi_free(its_dev->event_map.lpi_map, its_dev->event_map.lpi_base, @@ -2593,6 +2612,8 @@ static void its_irq_domain_free(struct i its_free_device(its_dev); }
+ mutex_unlock(&its->dev_alloc_lock); + irq_domain_free_irqs_parent(domain, virq, nr_irqs); }
@@ -3517,6 +3538,7 @@ static int __init its_probe_one(struct r }
raw_spin_lock_init(&its->lock); + mutex_init(&its->dev_alloc_lock); INIT_LIST_HEAD(&its->entry); INIT_LIST_HEAD(&its->its_device_list); typer = gic_read_typer(its_base + GITS_TYPER);