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@xxxxxxxxxx>
> Signed-off-by: Wang Hai <wanghai38@xxxxxxxxxx>
> Acked-by: Barry Song <song.bao.hua@xxxxxxxxxxxxx>
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
>
