Commit f98d67ac authored by Andy Shevchenko's avatar Andy Shevchenko Committed by Hans de Goede
Browse files

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: 9745fb07 ("platform/x86/intel: Add Primary to Sideband (P2SB) bridge support")
Reported-by: default avatarkernel test robot <oliver.sang@intel.com>
Suggested-by: default avatarMika Westerberg <mika.westerberg@linux.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: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20220901113406.65876-1-andriy.shevchenko@linux.intel.com


Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
parent 134038b0

drivers/platform/x86/p2sb.c

+16 −2
Original line number Diff line number Diff line
@@ -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;

}