Re: [PATCH] PCI/MSI: Fix UAF in msi_capability_init

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, Apr 29 2024 at 03:41, Mostafa Saleh wrote:
>  err:
> -	pci_msi_unmask(entry, msi_multi_mask(entry));
> +	/* Re-read the descriptor as it might have been freed */
> +	entry = msi_first_desc(&dev->dev, MSI_DESC_ALL);
> +	if (entry)
> +		pci_msi_unmask(entry, msi_multi_mask(entry));

What unmasks the entry in the NULL case?

The mask has to be undone. So you need something like the uncompiled
below.

Thanks,

        tglx
---

--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -349,7 +349,7 @@ static int msi_capability_init(struct pc
 			       struct irq_affinity *affd)
 {
 	struct irq_affinity_desc *masks = NULL;
-	struct msi_desc *entry;
+	struct msi_desc *entry, desc;
 	int ret;
 
 	/* Reject multi-MSI early on irq domain enabled architectures */
@@ -374,6 +374,12 @@ static int msi_capability_init(struct pc
 	/* All MSIs are unmasked by default; mask them all */
 	entry = msi_first_desc(&dev->dev, MSI_DESC_ALL);
 	pci_msi_mask(entry, msi_multi_mask(entry));
+	/*
+	 * Copy the MSI descriptor for the error path because
+	 * pci_msi_setup_msi_irqs() will free it for the hierarchical
+	 * interrupt domain case.
+	 */
+	memcpy(&desc, entry, sizeof(desc));
 
 	/* Configure MSI capability structure */
 	ret = pci_msi_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI);
@@ -393,7 +399,7 @@ static int msi_capability_init(struct pc
 	goto unlock;
 
 err:
-	pci_msi_unmask(entry, msi_multi_mask(entry));
+	pci_msi_unmask(&desc, msi_multi_mask(&desc));
 	pci_free_msi_irqs(dev);
 fail:
 	dev->msi_enabled = 0;