linux-kernel - Re: [PATCH] PCI/MSI: Fix UAF in msi_capability

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87zftbswwo.ffs@tglx>
Date: Mon, 29 Apr 2024 23:04:39 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Mostafa Saleh <smostafa@...gle.com>, bhelgaas@...gle.com,
 linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: ilpo.jarvinen@...ux.intel.com, kernel-team@...roid.com, Mostafa Saleh
 <smostafa@...gle.com>
Subject: Re: [PATCH] PCI/MSI: Fix UAF in msi_capability_init

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;