Linux USB
 help / color / mirror / Atom feed
* [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
@ 2026-08-07  8:37 Dmitry Antipov
  2026-08-07  9:58 ` Mika Westerberg
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Antipov @ 2026-08-07  8:37 UTC (permalink / raw)
  To: Andreas Noever, Mika Westerberg, Yehezkel Bernat
  Cc: Greg Kroah-Hartman, linux-usb, lvc-project, Dmitry Antipov,
	syzbot+901ca72278dfd89daf58

Syzbot reproducer at [1] enforces the kernel to probe PCI device 00:02.0
as Thunderbolt NHI. On QEMU/aarch64 'virt' machine, the device (at least
with qemu >= 11.0.0) is:

00:02.0 Class 0100: Device 1af4:1001
	Subsystem: Device 1af4:0002
	Flags: bus master, fast devsel, latency 0, IRQ 47
	I/O ports at 1000 [size=128]                           <-- Hmmm...
	Memory at 10041000 (32-bit, non-prefetchable) [size=4K]
	Memory at 8000004000 (64-bit, prefetchable) [size=16K]

So call to 'pcim_iomap_region(pdev, 0, ...)' in 'nhi_pci_probe()' maps this
128-bytes I/O ports area, and call to 'ioread32(nhi->iobase + REG_CAPS)' in
'nhi_probe()' issues an invalid access at REG_CAPS (0x39640) offset. Since
NHI's typical register window size is 256K, simple sanity check whether 1)
the region is a memory rather than I/O ports and 2) the region is 256K at
least should be enough to prevent from such a scenario.

[1] https://syzkaller.appspot.com/text?tag=ReproC&x=1564acc6580000

Reported-by: syzbot+901ca72278dfd89daf58@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=901ca72278dfd89daf58
Fixes: 16603153666d ("thunderbolt: Add initial cactus ridge NHI support")
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/thunderbolt/nhi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 0f795ea58756..724263c17b3e 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -1186,6 +1186,7 @@ static struct tb *nhi_select_cm(struct tb_nhi *nhi)
 int nhi_probe(struct tb_nhi *nhi)
 {
 	struct device *dev = nhi->dev;
+	struct pci_dev *pdev;
 	struct tb *tb;
 	int res;
 
@@ -1195,6 +1196,12 @@ int nhi_probe(struct tb_nhi *nhi)
 	if (!nhi->ops->init_interrupts)
 		return dev_err_probe(dev, -EINVAL, "missing required NHI ops\n");
 
+	pdev = to_pci_dev(dev);
+	if (!pci_resource_is_mem(pdev, 0))
+		return dev_err_probe(dev, -ENODEV, "invalid resource type\n");
+	if (pci_resource_len(pdev, 0) < 0x40000)
+		return dev_err_probe(dev, -ENODEV, "invalid resource size\n");
+
 	nhi->hop_count = ioread32(nhi->iobase + REG_CAPS) & 0x3ff;
 	dev_dbg(dev, "total paths: %d\n", nhi->hop_count);
 
-- 
2.55.0


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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07  8:37 [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe() Dmitry Antipov
@ 2026-08-07  9:58 ` Mika Westerberg
  2026-08-07 11:13   ` Dmitry Antipov
  0 siblings, 1 reply; 11+ messages in thread
From: Mika Westerberg @ 2026-08-07  9:58 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

Hi,

On Fri, Aug 07, 2026 at 11:37:57AM +0300, Dmitry Antipov wrote:
> Syzbot reproducer at [1] enforces the kernel to probe PCI device 00:02.0
> as Thunderbolt NHI. On QEMU/aarch64 'virt' machine, the device (at least
> with qemu >= 11.0.0) is:
> 
> 00:02.0 Class 0100: Device 1af4:1001
> 	Subsystem: Device 1af4:0002
> 	Flags: bus master, fast devsel, latency 0, IRQ 47
> 	I/O ports at 1000 [size=128]                           <-- Hmmm...
> 	Memory at 10041000 (32-bit, non-prefetchable) [size=4K]
> 	Memory at 8000004000 (64-bit, prefetchable) [size=16K]

Yeah, I'm not entirely sure we want to start "fixing" issues like these
where it's clearly not a USB4/TB NHI. IMHO If user forces the driver
somehow to bind to this unrelated device then she/he got what asked for.

> So call to 'pcim_iomap_region(pdev, 0, ...)' in 'nhi_pci_probe()' maps this
> 128-bytes I/O ports area, and call to 'ioread32(nhi->iobase + REG_CAPS)' in
> 'nhi_probe()' issues an invalid access at REG_CAPS (0x39640) offset. Since
> NHI's typical register window size is 256K, simple sanity check whether 1)
> the region is a memory rather than I/O ports and 2) the region is 256K at
> least should be enough to prevent from such a scenario.
> 
> [1] https://syzkaller.appspot.com/text?tag=ReproC&x=1564acc6580000
> 
> Reported-by: syzbot+901ca72278dfd89daf58@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=901ca72278dfd89daf58
> Fixes: 16603153666d ("thunderbolt: Add initial cactus ridge NHI support")
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/thunderbolt/nhi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 0f795ea58756..724263c17b3e 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -1186,6 +1186,7 @@ static struct tb *nhi_select_cm(struct tb_nhi *nhi)
>  int nhi_probe(struct tb_nhi *nhi)
>  {
>  	struct device *dev = nhi->dev;
> +	struct pci_dev *pdev;
>  	struct tb *tb;
>  	int res;
>  
> @@ -1195,6 +1196,12 @@ int nhi_probe(struct tb_nhi *nhi)
>  	if (!nhi->ops->init_interrupts)
>  		return dev_err_probe(dev, -EINVAL, "missing required NHI ops\n");
>  
> +	pdev = to_pci_dev(dev);
> +	if (!pci_resource_is_mem(pdev, 0))
> +		return dev_err_probe(dev, -ENODEV, "invalid resource type\n");
> +	if (pci_resource_len(pdev, 0) < 0x40000)
> +		return dev_err_probe(dev, -ENODEV, "invalid resource size\n");
> +
>  	nhi->hop_count = ioread32(nhi->iobase + REG_CAPS) & 0x3ff;
>  	dev_dbg(dev, "total paths: %d\n", nhi->hop_count);
>  
> -- 
> 2.55.0

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07  9:58 ` Mika Westerberg
@ 2026-08-07 11:13   ` Dmitry Antipov
  2026-08-07 11:31     ` Mika Westerberg
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Antipov @ 2026-08-07 11:13 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

On 8/7/26 12:58 PM, Mika Westerberg wrote:

> Yeah, I'm not entirely sure we want to start "fixing" issues like these
> where it's clearly not a USB4/TB NHI. IMHO If user forces the driver
> somehow to bind to this unrelated device then she/he got what asked for.

I agree but isn't there a scenario to trigger such a weird probe just by mistake?

Dmitry

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 11:13   ` Dmitry Antipov
@ 2026-08-07 11:31     ` Mika Westerberg
  2026-08-07 11:47       ` Dmitry Antipov
  0 siblings, 1 reply; 11+ messages in thread
From: Mika Westerberg @ 2026-08-07 11:31 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

On Fri, Aug 07, 2026 at 02:13:06PM +0300, Dmitry Antipov wrote:
> On 8/7/26 12:58 PM, Mika Westerberg wrote:
> 
> > Yeah, I'm not entirely sure we want to start "fixing" issues like these
> > where it's clearly not a USB4/TB NHI. IMHO If user forces the driver
> > somehow to bind to this unrelated device then she/he got what asked for.
> 
> I agree but isn't there a scenario to trigger such a weird probe just by mistake?

Okay but how it is "mistake"? ;-) I'm actually not sure how this was even
triggered. Something like you as root echo random PCI device name to "bind"
files of a driver?

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 11:31     ` Mika Westerberg
@ 2026-08-07 11:47       ` Dmitry Antipov
  2026-08-07 12:01         ` Mika Westerberg
  2026-08-07 13:11         ` Greg Kroah-Hartman
  0 siblings, 2 replies; 11+ messages in thread
From: Dmitry Antipov @ 2026-08-07 11:47 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

On 8/7/26 2:31 PM, Mika Westerberg wrote:

> I'm actually not sure how this was even triggered

:-) just as documented in https://docs.kernel.org/driver-api/driver-model/binding.html:

Userspace may override the standard matching by writing a driver name to a device’s
driver_override sysfs attribute. When set, only a driver whose name matches the
override will be considered during binding. This bypasses all bus-specific matching
(OF, ACPI, ID tables, etc.).

Dmitry

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 11:47       ` Dmitry Antipov
@ 2026-08-07 12:01         ` Mika Westerberg
  2026-08-07 12:36           ` Dmitry Antipov
  2026-08-07 13:11         ` Greg Kroah-Hartman
  1 sibling, 1 reply; 11+ messages in thread
From: Mika Westerberg @ 2026-08-07 12:01 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

On Fri, Aug 07, 2026 at 02:47:54PM +0300, Dmitry Antipov wrote:
> On 8/7/26 2:31 PM, Mika Westerberg wrote:
> 
> > I'm actually not sure how this was even triggered
> 
> :-) just as documented in https://docs.kernel.org/driver-api/driver-model/binding.html:
> 
> Userspace may override the standard matching by writing a driver name to a device’s
> driver_override sysfs attribute. When set, only a driver whose name matches the
> override will be considered during binding. This bypasses all bus-specific matching
> (OF, ACPI, ID tables, etc.).

Okay through driver_override, thanks. It also says:

  Buses opt into this mechanism by setting the driver_override flag in their
  struct bus_type.

But that's not done in struct tb_bust_type.

So there should be no such attribute available even.

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 12:01         ` Mika Westerberg
@ 2026-08-07 12:36           ` Dmitry Antipov
  2026-08-07 13:05             ` Mika Westerberg
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Antipov @ 2026-08-07 12:36 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

On 8/7/26 3:01 PM, Mika Westerberg wrote:

> Okay through driver_override, thanks. It also says:
> 
>    Buses opt into this mechanism by setting the driver_override flag in their
>    struct bus_type.
> 
> But that's not done in struct tb_bus_type.
> 
> So there should be no such attribute available even.

Hm. Syzbot's reproducer definitely uses /sys/bus/pci/devices/0000:00:02.0/driver_override
and /sys/bus/pci/drivers/thunderbolt/bind to make a trick. IIUC the kernel starts to probe
NHI just like any regular PCI device (so driver_override is expected to work), and
tb_bus_type enters the game during the probe itself during nhi_probe() -> nhi_select_cm()
-> tb_probe() -> tb_domain_alloc().

Dmitry


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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 12:36           ` Dmitry Antipov
@ 2026-08-07 13:05             ` Mika Westerberg
  0 siblings, 0 replies; 11+ messages in thread
From: Mika Westerberg @ 2026-08-07 13:05 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	Greg Kroah-Hartman, linux-usb, lvc-project,
	syzbot+901ca72278dfd89daf58

On Fri, Aug 07, 2026 at 03:36:09PM +0300, Dmitry Antipov wrote:
> On 8/7/26 3:01 PM, Mika Westerberg wrote:
> 
> > Okay through driver_override, thanks. It also says:
> > 
> >    Buses opt into this mechanism by setting the driver_override flag in their
> >    struct bus_type.
> > 
> > But that's not done in struct tb_bus_type.
> > 
> > So there should be no such attribute available even.
> 
> Hm. Syzbot's reproducer definitely uses /sys/bus/pci/devices/0000:00:02.0/driver_override
> and /sys/bus/pci/drivers/thunderbolt/bind to make a trick. IIUC the kernel starts to probe
> NHI just like any regular PCI device (so driver_override is expected to work), and
> tb_bus_type enters the game during the probe itself during nhi_probe() -> nhi_select_cm()
> -> tb_probe() -> tb_domain_alloc().

Well say if it finds a random device that has BAR that is of correct type
and size then we are in the same situation again (and I think this applies
to many drivers -- that's why we have the ID/class or similar matching
there to make sure these bind to expected hardware).

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 11:47       ` Dmitry Antipov
  2026-08-07 12:01         ` Mika Westerberg
@ 2026-08-07 13:11         ` Greg Kroah-Hartman
  2026-08-07 13:27           ` Dmitry Antipov
  1 sibling, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-07 13:11 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Mika Westerberg, Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	linux-usb, lvc-project, syzbot+901ca72278dfd89daf58

On Fri, Aug 07, 2026 at 02:47:54PM +0300, Dmitry Antipov wrote:
> On 8/7/26 2:31 PM, Mika Westerberg wrote:
> 
> > I'm actually not sure how this was even triggered
> 
> :-) just as documented in https://docs.kernel.org/driver-api/driver-model/binding.html:
> 
> Userspace may override the standard matching by writing a driver name to a device’s
> driver_override sysfs attribute. When set, only a driver whose name matches the
> override will be considered during binding. This bypasses all bus-specific matching
> (OF, ACPI, ID tables, etc.).

Yes, and anyone who uses this, gets to keep the broken pieces of the
kernel if they attempt to bind a driver to a device that can't actually
support it.

This is a debugging-only feature, don't treat it as something for which
EVERY driver would have to somehow insulate themselves from.  That's not
what this is for at all.

thanks,

greg k-h

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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 13:11         ` Greg Kroah-Hartman
@ 2026-08-07 13:27           ` Dmitry Antipov
  2026-08-07 13:59             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Antipov @ 2026-08-07 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mika Westerberg, Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	linux-usb, lvc-project, syzbot+901ca72278dfd89daf58

On 8/7/26 4:11 PM, Greg Kroah-Hartman wrote:

> This is a debugging-only feature

AFAIK QEMU's vfio-pci uses this to bypass PCI devices to guests.

Dmitry


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

* Re: [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe()
  2026-08-07 13:27           ` Dmitry Antipov
@ 2026-08-07 13:59             ` Greg Kroah-Hartman
  0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-07 13:59 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Mika Westerberg, Andreas Noever, Mika Westerberg, Yehezkel Bernat,
	linux-usb, lvc-project, syzbot+901ca72278dfd89daf58

On Fri, Aug 07, 2026 at 04:27:09PM +0300, Dmitry Antipov wrote:
> On 8/7/26 4:11 PM, Greg Kroah-Hartman wrote:
> 
> > This is a debugging-only feature
> 
> AFAIK QEMU's vfio-pci uses this to bypass PCI devices to guests.

And it gets to keep the broken pieces that will happen when things go
wrong as it is explicitly saying "I know what I am doing!" yet it
doesn't :)

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

end of thread, other threads:[~2026-08-07 14:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07  8:37 [PATCH] thunderbolt: verify PCI resource type and size in nhi_probe() Dmitry Antipov
2026-08-07  9:58 ` Mika Westerberg
2026-08-07 11:13   ` Dmitry Antipov
2026-08-07 11:31     ` Mika Westerberg
2026-08-07 11:47       ` Dmitry Antipov
2026-08-07 12:01         ` Mika Westerberg
2026-08-07 12:36           ` Dmitry Antipov
2026-08-07 13:05             ` Mika Westerberg
2026-08-07 13:11         ` Greg Kroah-Hartman
2026-08-07 13:27           ` Dmitry Antipov
2026-08-07 13:59             ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox