| Message ID | 20211012071556.939137-1-wanghai38@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2b94b6b79b7c24092a6169db9e83c4565be0db42 |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v3] PCI/MSI: fix page fault when msi_populate_sysfs() failed | expand |
On Tue, Oct 12, 2021 at 03:15:56PM +0800, Wang Hai wrote: > I got a page fault report when doing fault injection test: > > BUG: unable to handle page fault for address: fffffffffffffff4 > ... > RIP: 0010:sysfs_remove_groups+0x25/0x60 > ... > Call Trace: > msi_destroy_sysfs+0x30/0xa0 > free_msi_irqs+0x11d/0x1b0 > __pci_enable_msix_range+0x67f/0x760 > pci_alloc_irq_vectors_affinity+0xe7/0x170 > vp_find_vqs_msix+0x129/0x560 > vp_find_vqs+0x52/0x230 > vp_modern_find_vqs+0x47/0xb0 > p9_virtio_probe+0xa1/0x460 [9pnet_virtio] > ... > entry_SYSCALL_64_after_hwframe+0x44/0xae > > When populating msi_irqs sysfs failed (such as msi_attrs = kcalloc() > in msi_populate_sysfs() failed) in msi_capability_init() or > msix_capability_init(), dev->msi_irq_groups will point to ERR_PTR(...). > This will cause a page fault when destroying the wrong > dev->msi_irq_groups in free_msi_irqs(). > > msix_capability_init()/msi_capability_init() > msi_populate_sysfs() > msi_attrs = kcalloc() // fault injection, let msi_attrs = NULL > free_msi_irqs() > msi_destroy_sysfs() // msi_irq_groups is ERR_PTR(...), page fault > > Define a temp variable and assign it to dev->msi_irq_groups if > the temp variable is not PTR_ERR. > > Fixes: 2f170814bdd2 ("genirq/msi: Move MSI sysfs handling from PCI to MSI core") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Wang Hai <wanghai38@huawei.com> > Acked-by: Barry Song <song.bao.hua@hisilicon.com> 2f170814bdd2 appeared in v5.15-rc1, so I applied this to for-linus so we can fix it before v5.15. Thank you! > --- > v2->v3: refine the commit log > v1->v2: introduce temporary variable 'groups' > drivers/pci/msi.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index 0099a00af361..4b4792940e86 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -535,6 +535,7 @@ static int msi_verify_entries(struct pci_dev *dev) > static int msi_capability_init(struct pci_dev *dev, int nvec, > struct irq_affinity *affd) > { > + const struct attribute_group **groups; > struct msi_desc *entry; > int ret; > > @@ -558,12 +559,14 @@ static int msi_capability_init(struct pci_dev *dev, int nvec, > if (ret) > goto err; > > - dev->msi_irq_groups = msi_populate_sysfs(&dev->dev); > - if (IS_ERR(dev->msi_irq_groups)) { > - ret = PTR_ERR(dev->msi_irq_groups); > + groups = msi_populate_sysfs(&dev->dev); > + if (IS_ERR(groups)) { > + ret = PTR_ERR(groups); > goto err; > } > > + dev->msi_irq_groups = groups; > + > /* Set MSI enabled bits */ > pci_intx_for_msi(dev, 0); > pci_msi_set_enable(dev, 1); > @@ -691,6 +694,7 @@ static void msix_mask_all(void __iomem *base, int tsize) > static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, > int nvec, struct irq_affinity *affd) > { > + const struct attribute_group **groups; > void __iomem *base; > int ret, tsize; > u16 control; > @@ -730,12 +734,14 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, > > msix_update_entries(dev, entries); > > - dev->msi_irq_groups = msi_populate_sysfs(&dev->dev); > - if (IS_ERR(dev->msi_irq_groups)) { > - ret = PTR_ERR(dev->msi_irq_groups); > + groups = msi_populate_sysfs(&dev->dev); > + if (IS_ERR(groups)) { > + ret = PTR_ERR(groups); > goto out_free; > } > > + dev->msi_irq_groups = groups; > + > /* Set MSI-X enabled bits and unmask the function */ > pci_intx_for_msi(dev, 0); > dev->msix_enabled = 1; > -- > 2.17.1 >
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 0099a00af361..4b4792940e86 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -535,6 +535,7 @@ static int msi_verify_entries(struct pci_dev *dev) static int msi_capability_init(struct pci_dev *dev, int nvec, struct irq_affinity *affd) { + const struct attribute_group **groups; struct msi_desc *entry; int ret; @@ -558,12 +559,14 @@ static int msi_capability_init(struct pci_dev *dev, int nvec, if (ret) goto err; - dev->msi_irq_groups = msi_populate_sysfs(&dev->dev); - if (IS_ERR(dev->msi_irq_groups)) { - ret = PTR_ERR(dev->msi_irq_groups); + groups = msi_populate_sysfs(&dev->dev); + if (IS_ERR(groups)) { + ret = PTR_ERR(groups); goto err; } + dev->msi_irq_groups = groups; + /* Set MSI enabled bits */ pci_intx_for_msi(dev, 0); pci_msi_set_enable(dev, 1); @@ -691,6 +694,7 @@ static void msix_mask_all(void __iomem *base, int tsize) static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, int nvec, struct irq_affinity *affd) { + const struct attribute_group **groups; void __iomem *base; int ret, tsize; u16 control; @@ -730,12 +734,14 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, msix_update_entries(dev, entries); - dev->msi_irq_groups = msi_populate_sysfs(&dev->dev); - if (IS_ERR(dev->msi_irq_groups)) { - ret = PTR_ERR(dev->msi_irq_groups); + groups = msi_populate_sysfs(&dev->dev); + if (IS_ERR(groups)) { + ret = PTR_ERR(groups); goto out_free; } + dev->msi_irq_groups = groups; + /* Set MSI-X enabled bits and unmask the function */ pci_intx_for_msi(dev, 0); dev->msix_enabled = 1;
I got a page fault report when doing fault injection test: BUG: unable to handle page fault for address: fffffffffffffff4 ... RIP: 0010:sysfs_remove_groups+0x25/0x60 ... Call Trace: msi_destroy_sysfs+0x30/0xa0 free_msi_irqs+0x11d/0x1b0 __pci_enable_msix_range+0x67f/0x760 pci_alloc_irq_vectors_affinity+0xe7/0x170 vp_find_vqs_msix+0x129/0x560 vp_find_vqs+0x52/0x230 vp_modern_find_vqs+0x47/0xb0 p9_virtio_probe+0xa1/0x460 [9pnet_virtio] ... entry_SYSCALL_64_after_hwframe+0x44/0xae When populating msi_irqs sysfs failed (such as msi_attrs = kcalloc() in msi_populate_sysfs() failed) in msi_capability_init() or msix_capability_init(), dev->msi_irq_groups will point to ERR_PTR(...). This will cause a page fault when destroying the wrong dev->msi_irq_groups in free_msi_irqs(). msix_capability_init()/msi_capability_init() msi_populate_sysfs() msi_attrs = kcalloc() // fault injection, let msi_attrs = NULL free_msi_irqs() msi_destroy_sysfs() // msi_irq_groups is ERR_PTR(...), page fault Define a temp variable and assign it to dev->msi_irq_groups if the temp variable is not PTR_ERR. Fixes: 2f170814bdd2 ("genirq/msi: Move MSI sysfs handling from PCI to MSI core") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Wang Hai <wanghai38@huawei.com> Acked-by: Barry Song <song.bao.hua@hisilicon.com> --- v2->v3: refine the commit log v1->v2: introduce temporary variable 'groups' drivers/pci/msi.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)