linux-kernel - [PATCH v1 1/1] platform/x86: p2sb: Fix UAF when caller uses resource name

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-next>] [day] [month] [year] [list]

Date:   Thu,  1 Sep 2022 14:34:06 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Mark Gross <markgross@...nel.org>,
        kernel test robot <oliver.sang@...el.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: [PATCH v1 1/1] platform/x86: p2sb: Fix UAF when caller uses resource name

We have to copy only selected fields from the original resource.
Because a PCI device will be removed immediately after getting
its resources, we may not use any allocated data, hence we may
not copy any pointers.

Consider the following scenario:

  1/ a caller of p2sb_bar() gets the resource;

  2/ the resource has been copied by platform_device_add_data()
     in order to create a platform device;

  3/ the platform device creation will call for the device driver's
     ->probe() as soon as a match found;

  4/ the ->probe() takes given resources (see 2/) and tries to
     access one of its field, i.e. 'name', in the
     __devm_ioremap_resource() to create a pretty looking output;

  5/ but the 'name' is a dangling pointer because p2sb_bar()
     removed a PCI device, which 'name' had been copied to
     the caller's memory.

  6/ UAF (Use-After-Free) as a result.

Kudos to Mika for the initial analisys of the issue.

Fixes: 9745fb07474f ("platform/x86/intel: Add Primary to Sideband (P2SB) bridge support")
Reported-by: kernel test robot <oliver.sang@...el.com>
Suggested-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
Link: https://lore.kernel.org/linux-i2c/YvPCbnKqDiL2XEKp@xsang-OptiPlex-9020/
Link: https://lore.kernel.org/linux-i2c/YtjAswDKfiuDfWYs@xsang-OptiPlex-9020/
Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
---
 drivers/platform/x86/p2sb.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c
index fb2e141f3eb8..384d0962ae93 100644
--- a/drivers/platform/x86/p2sb.c
+++ b/drivers/platform/x86/p2sb.c
@@ -42,10 +42,24 @@ static int p2sb_get_devfn(unsigned int *devfn)
 	return 0;
 }
 
+/* Copy resource from the first BAR of the device in question */
 static int p2sb_read_bar0(struct pci_dev *pdev, struct resource *mem)
 {
-	/* Copy resource from the first BAR of the device in question */
-	*mem = pdev->resource[0];
+	struct resource *bar0 = &pdev->resource[0];
+
+	/* Make sure we have no dangling pointers in the output */
+	memset(mem, 0, sizeof(*mem));
+
+	/*
+	 * We copy only selected fields from the original resource.
+	 * Because a PCI device will be removed soon, we may not use
+	 * any allocated data, hence we may not copy any pointers.
+	 */
+	mem->start = bar0->start;
+	mem->end = bar0->end;
+	mem->flags = bar0->flags;
+	mem->desc = bar0->desc;
+
 	return 0;
 }
 
-- 
2.35.1