All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping
@ 2026-09-14  9:50 Slawomir Stepien
  2026-09-14 10:04 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Slawomir Stepien @ 2026-09-14  9:50 UTC (permalink / raw)
  To: syzkaller-bugs, Basavaraj Natikar, bentiss, jikos, linux-input,
	sandeep.singh
  Cc: linux-kernel, syzbot, Slawomir Stepien,
	syzbot+4eadd4dfe9e66522bae8

The amd_sfh driver maps PCI BAR 2 using pcim_iomap_regions() and
subsequently accesses MMIO registers at offsets up to 0x10958 (e.g.,
AMD_P2C_MSG3 at 0x1068C). However, the driver never validates that the BAR
size is large enough to cover these accesses. If the driver is bound to a
device with a smaller BAR 2, this leads to an out-of-bounds memory access
and a page fault during the probe function.

For example, a page fault can occur when reading from privdata->mmio +
AMD_P2C_MSG3 in mp2_select_ops():

  BUG: unable to handle page fault for address: ffffc9000390368c
  PGD 100000067 P4D 100000067 PUD 1012c1067 PMD 105b64067 PTE 0
  Oops: Oops: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:readl arch/x86/include/asm/io.h:59 [inline]
  RIP: 0010:mp2_select_ops drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:282
  [inline]
  RIP: 0010:amd_mp2_pci_probe+0x337/0x5f0
  drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:487
  Call Trace:
   <TASK>
   local_pci_probe drivers/pci/pci-driver.c:332 [inline]
   pci_call_probe drivers/pci/pci-driver.c:394 [inline]
   __pci_device_probe drivers/pci/pci-driver.c:455 [inline]
   pci_device_probe+0x431/0xc90 drivers/pci/pci-driver.c:489

Fix this by verifying that the length of BAR 2 is at least 128KB before
attempting to map it. Since the maximum accessed offset is 0x10958, and PCI
BAR sizes are powers of 2, any legitimate hardware will have a BAR size of
at least 128KB.

Fixes: 4f567b9f8141 ("SFH: PCIe driver to add support of AMD sensor fusion hub")
Assisted-by: Gemini:gemini-3.7-flash Gemini:gemini-3.1-pro-preview syzbot
Reported-by: syzbot+4eadd4dfe9e66522bae8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4eadd4dfe9e66522bae8
Link: https://syzkaller.appspot.com/ai_job?id=3bc1c45c-548f-4ab5-8243-d2c8ec321d6c
Signed-off-by: Slawomir Stepien <sst@poczta.fm>
Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
v2:
* Use SZ_128K for AMD_SFH_MIN_BAR_SIZE define
* Add comment why we need 128K
* Add 'Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>' since no logical changes

v1:
* https://lore.kernel.org/all/94704655-d654-4500-8122-b80bea9bd908@mail.kernel.org/
---
 drivers/hid/amd-sfh-hid/amd_sfh_common.h |  4 ++++
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c   | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index f8c6b7fc34fb..3d29119a3765 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -13,11 +13,15 @@
 #include <linux/auxiliary_bus.h>
 #include <linux/mutex.h>
 #include <linux/pci.h>
+#include <linux/sizes.h>
 #include "amd_sfh_hid.h"
 
 #define PCI_DEVICE_ID_AMD_MP2		0x15E4
 #define PCI_DEVICE_ID_AMD_MP2_1_1	0x164A
 
+/* The BAR 2 size must cover the highest register offset (0x10958) */
+#define AMD_SFH_MIN_BAR_SIZE		SZ_128K
+
 #define AMD_C2P_MSG(regno) (0x10500 + ((regno) * 4))
 #define AMD_P2C_MSG(regno) (0x10680 + ((regno) * 4))
 
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index eda26a094d3f..061a63519d44 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -497,6 +497,16 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	if (rc)
 		return rc;
 
+	if (!(pci_resource_flags(pdev, 2) & IORESOURCE_MEM)) {
+		dev_err(&pdev->dev, "BAR 2 is not IORESOURCE_MEM\n");
+		return -ENODEV;
+	}
+
+	if (pci_resource_len(pdev, 2) < AMD_SFH_MIN_BAR_SIZE) {
+		dev_err(&pdev->dev, "BAR 2 is too small\n");
+		return -EINVAL;
+	}
+
 	rc = pcim_iomap_regions(pdev, BIT(2), DRIVER_NAME);
 	if (rc)
 		return rc;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping
  2026-09-14  9:50 [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping Slawomir Stepien
@ 2026-09-14 10:04 ` sashiko-bot
  2026-09-14 12:42 ` Jiri Kosina
  2026-09-14 13:18 ` [syzbot ci] " syzbot ci
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-14 10:04 UTC (permalink / raw)
  To: Slawomir Stepien; +Cc: dmitry.torokhov, linux-input

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data race on `cli_data->cur_hid_dev` in `amd_sfh_set_report()` can lead to wrong completion flags being set, causing legitimate HID requests to time out.
--

commit eb712409bb933989aa1a882596c6ff02ffcdb00c
Author: Slawomir Stepien <sst@poczta.fm>
Subject: HID: amd_sfh: Validate PCI BAR size before mapping

This patch validates that the PCI BAR 2 size is at least 128KB before
attempting to map it. This prevents an out-of-bounds memory access and page
fault during the probe function if the driver is bound to a device with a
smaller BAR size.

Link: https://syzkaller.appspot.com/bug?extid=4eadd4dfe9e66522bae8
Link: https://syzkaller.appspot.com/ai_job?id=3bc1c45c-548f-4ab5-8243-d2c8ec321d6c

> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index eda26a094d3f1..061a63519d441 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
[ ... ]
> @@ -497,6 +497,16 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
>  	if (rc)
>  		return rc;
>  
> +	if (!(pci_resource_flags(pdev, 2) & IORESOURCE_MEM)) {
> +		dev_err(&pdev->dev, "BAR 2 is not IORESOURCE_MEM\n");
> +		return -ENODEV;
> +	}
> +
> +	if (pci_resource_len(pdev, 2) < AMD_SFH_MIN_BAR_SIZE) {
> +		dev_err(&pdev->dev, "BAR 2 is too small\n");
> +		return -EINVAL;
> +	}
> +
>  	rc = pcim_iomap_regions(pdev, BIT(2), DRIVER_NAME);
>  	if (rc)
>  		return rc;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible to have a data
race on cli_data->cur_hid_dev here in the amd_sfh driver?

Looking at drivers/hid/amd-sfh-hid/amd_sfh_client.c:amd_sfh_set_report(),
the shared global state is modified without acquiring the mp2->lock mutex:

amd_sfh_set_report() {
    ...
    cli_data->cur_hid_dev = i;
    break;
    ...
}

Could this lead to wrong completion flags being set in amdtp_hid_wakeup()?

amdtp_hid_wakeup() {
    ...
    cli_data->request_done[cli_data->cur_hid_dev] = true;
    ...
}

If Process A triggers HID_REQ_GET_REPORT on one HID device, it queues work to
amd_sfh_work(). Concurrently, if Process B triggers HID_REQ_SET_REPORT via
ioctl on a different HID device on the same AMD SFH PCI controller,
amd_sfh_set_report() modifies cli_data->cur_hid_dev without lock protection.

Would this cause amdtp_hid_wakeup() to read the racy cur_hid_dev value, write
to the wrong request_done index, and wake up the wrong waitqueue, causing
legitimate HID requests to time out?

This seems reachable by local users capable of sending HID raw requests (e.g.,
via /dev/hidrawX), which in many desktop configurations is accessible to the
logged-in user.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914095007.205287-1-sst@poczta.fm?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping
  2026-09-14  9:50 [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping Slawomir Stepien
  2026-09-14 10:04 ` sashiko-bot
@ 2026-09-14 12:42 ` Jiri Kosina
  2026-09-14 13:18 ` [syzbot ci] " syzbot ci
  2 siblings, 0 replies; 4+ messages in thread
From: Jiri Kosina @ 2026-09-14 12:42 UTC (permalink / raw)
  To: Slawomir Stepien
  Cc: syzkaller-bugs, Basavaraj Natikar, bentiss, linux-input,
	sandeep.singh, linux-kernel, syzbot, syzbot+4eadd4dfe9e66522bae8

On Mon, 14 Sep 2026, Slawomir Stepien wrote:

> The amd_sfh driver maps PCI BAR 2 using pcim_iomap_regions() and
> subsequently accesses MMIO registers at offsets up to 0x10958 (e.g.,
> AMD_P2C_MSG3 at 0x1068C). However, the driver never validates that the BAR
> size is large enough to cover these accesses. If the driver is bound to a
> device with a smaller BAR 2, this leads to an out-of-bounds memory access
> and a page fault during the probe function.
> 
> For example, a page fault can occur when reading from privdata->mmio +
> AMD_P2C_MSG3 in mp2_select_ops():
> 
>   BUG: unable to handle page fault for address: ffffc9000390368c
>   PGD 100000067 P4D 100000067 PUD 1012c1067 PMD 105b64067 PTE 0
>   Oops: Oops: 0000 [#1] SMP KASAN NOPTI
>   RIP: 0010:readl arch/x86/include/asm/io.h:59 [inline]
>   RIP: 0010:mp2_select_ops drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:282
>   [inline]
>   RIP: 0010:amd_mp2_pci_probe+0x337/0x5f0
>   drivers/hid/amd-sfh-hid/amd_sfh_pcie.c:487
>   Call Trace:
>    <TASK>
>    local_pci_probe drivers/pci/pci-driver.c:332 [inline]
>    pci_call_probe drivers/pci/pci-driver.c:394 [inline]
>    __pci_device_probe drivers/pci/pci-driver.c:455 [inline]
>    pci_device_probe+0x431/0xc90 drivers/pci/pci-driver.c:489
> 
> Fix this by verifying that the length of BAR 2 is at least 128KB before
> attempting to map it. Since the maximum accessed offset is 0x10958, and PCI
> BAR sizes are powers of 2, any legitimate hardware will have a BAR size of
> at least 128KB.
> 
> Fixes: 4f567b9f8141 ("SFH: PCIe driver to add support of AMD sensor fusion hub")
> Assisted-by: Gemini:gemini-3.7-flash Gemini:gemini-3.1-pro-preview syzbot
> Reported-by: syzbot+4eadd4dfe9e66522bae8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4eadd4dfe9e66522bae8
> Link: https://syzkaller.appspot.com/ai_job?id=3bc1c45c-548f-4ab5-8243-d2c8ec321d6c
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
> v2:
> * Use SZ_128K for AMD_SFH_MIN_BAR_SIZE define
> * Add comment why we need 128K
> * Add 'Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>' since no logical changes
> 
> v1:
> * https://lore.kernel.org/all/94704655-d654-4500-8122-b80bea9bd908@mail.kernel.org/

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [syzbot ci] Re: HID: amd_sfh: Validate PCI BAR size before mapping
  2026-09-14  9:50 [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping Slawomir Stepien
  2026-09-14 10:04 ` sashiko-bot
  2026-09-14 12:42 ` Jiri Kosina
@ 2026-09-14 13:18 ` syzbot ci
  2 siblings, 0 replies; 4+ messages in thread
From: syzbot ci @ 2026-09-14 13:18 UTC (permalink / raw)
  To: sst; +Cc: syzbot, syzkaller-bugs

syzbot ci has tested the following series

[v2] HID: amd_sfh: Validate PCI BAR size before mapping
https://lore.kernel.org/all/20260914095007.205287-1-sst@poczta.fm
* [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping

and found no issues.

Full report is available here:
https://ci.syzbot.org/series/f90e00ac-1ce2-4f3c-8983-cb0b61a5ce97

***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-14 13:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14  9:50 [PATCH v2] HID: amd_sfh: Validate PCI BAR size before mapping Slawomir Stepien
2026-09-14 10:04 ` sashiko-bot
2026-09-14 12:42 ` Jiri Kosina
2026-09-14 13:18 ` [syzbot ci] " syzbot ci

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.