Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Linus Walleij]

On Mon, Sep 28, 2026 at 9:25 AM jiale yao <19888972804@163.com> wrote:

> I reproduced this deterministically with KASAN on arm64 QEMU.
> The SPI device has an unmatched compatible and a valid microchip,spi-present-mask property, and is then bound with:
>
> echo mcp23s08 > /sys/bus/spi/devices/spi0.0/driver_override
> echo spi0.0 > /sys/bus/spi/drivers/mcp23s08/bind

[KASAN splat from spi_get_device_match_data() returning NULL]

But hey look in the driver:

static const struct spi_device_id mcp23s08_ids[] = {
        { "mcp23s08", (kernel_ulong_t)&mcp23s08_spi },
        { "mcp23s17", (kernel_ulong_t)&mcp23s17_spi },
        { "mcp23s18", (kernel_ulong_t)&mcp23s18_spi },
        { }
};
MODULE_DEVICE_TABLE(spi, mcp23s08_ids);

Why doesn't the override find the right data from the match table?

This looks more like a bug in the SPI bus implementation,
surely the bus should match a driver_overrid and return a
valid match data from spi_get_device_match_data()?

I think this is just papering over the real issue, you need
to dig into the SPI bus implementation and see why this isn't
working.

Yours,
Linus Walleij

Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Andy Shevchenko]

On Thu, Oct 01, 2026 at 10:07:29PM +0200, Linus Walleij wrote:
> On Mon, Sep 28, 2026 at 9:25 AM jiale yao <19888972804@163.com> wrote:
> 
> > I reproduced this deterministically with KASAN on arm64 QEMU.
> > The SPI device has an unmatched compatible and a valid microchip,spi-present-mask property, and is then bound with:
> >
> > echo mcp23s08 > /sys/bus/spi/devices/spi0.0/driver_override
> > echo spi0.0 > /sys/bus/spi/drivers/mcp23s08/bind
> 
> [KASAN splat from spi_get_device_match_data() returning NULL]
> 
> But hey look in the driver:
> 
> static const struct spi_device_id mcp23s08_ids[] = {
>         { "mcp23s08", (kernel_ulong_t)&mcp23s08_spi },
>         { "mcp23s17", (kernel_ulong_t)&mcp23s17_spi },
>         { "mcp23s18", (kernel_ulong_t)&mcp23s18_spi },
>         { }
> };
> MODULE_DEVICE_TABLE(spi, mcp23s08_ids);
> 
> Why doesn't the override find the right data from the match table?
> 
> This looks more like a bug in the SPI bus implementation,
> surely the bus should match a driver_overrid and return a
> valid match data from spi_get_device_match_data()?
> 
> I think this is just papering over the real issue, you need
> to dig into the SPI bus implementation and see why this isn't
> working.

I believe this is the whole design of driver_override like this...
There is an attempt to allow drivers to forbid that feature.

-- 
With Best Regards,
Andy Shevchenko

Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Linus Walleij]

On Fri, Oct 2, 2026 at 8:59 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> I believe this is the whole design of driver_override like this...
> There is an attempt to allow drivers to forbid that feature.

I guess I don't have the right background to understand
the driver_override feature, it feels someone should explain
its virtues to me because I feel I am getting really angry
at it and it may not deserve that.

The entire name of the thing feels like debugfs-footgun
territory for example.

Yours,
Linus Walleij

Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Andy Shevchenko]

On Fri, Oct 02, 2026 at 11:52:41AM +0200, Linus Walleij wrote:
> On Fri, Oct 2, 2026 at 8:59 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > I believe this is the whole design of driver_override like this...
> > There is an attempt to allow drivers to forbid that feature.
> 
> I guess I don't have the right background to understand
> the driver_override feature, it feels someone should explain
> its virtues to me because I feel I am getting really angry
> at it and it may not deserve that.
> 
> The entire name of the thing feels like debugfs-footgun
> territory for example.

Yeah, I can't find neither a thing on LWN.net nor in the in-tree documentation.
The only useful piece of information is (in kernel-doc of struct bus_type):

  driver_override
    Set to true if this bus supports the driver_override mechanism, which
    allows userspace to force a specific driver to bind to a device via a sysfs
    attribute.

-- 
With Best Regards,
Andy Shevchenko

Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Linus Walleij]

On Fri, Oct 2, 2026 at 12:19 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, Oct 02, 2026 at 11:52:41AM +0200, Linus Walleij wrote:
> > On Fri, Oct 2, 2026 at 8:59 AM Andy Shevchenko

> > The entire name of the thing feels like debugfs-footgun
> > territory for example.
>
> Yeah, I can't find neither a thing on LWN.net nor in the in-tree documentation.
> The only useful piece of information is (in kernel-doc of struct bus_type):
>
>   driver_override
>     Set to true if this bus supports the driver_override mechanism, which
>     allows userspace to force a specific driver to bind to a device via a sysfs
>     attribute.

This whole thing is weird, but OK.

Since we have a ton of drivers depending on match data we either
have to patch them all to bail out if match data is NULL (like this
patch does) or, which is equivalent, opt out of driver_override
that much is certain.

What I don't get is what this is intended for. What is the use case?
The commit says this is for VFIO. Shouldn't it be opt-in and turned
on only for VFIO then?

Kim Phillips is listed as contract for the platform bus driver_override
so let's ask! Kim: what is this for?

All of the device tree drivers use the platform bus, what's yhe
Unique Selling Point of this for our devices?

Yours,
Linus Walleij

Re: RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Andy Shevchenko]

+Cc: Danilo
(as you were involved in cleaning this up in the past and being co-maintainer
 of driver core)

On Sun, Oct 04, 2026 at 01:06:07AM +0200, Linus Walleij wrote:
> On Fri, Oct 2, 2026 at 12:19 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Oct 02, 2026 at 11:52:41AM +0200, Linus Walleij wrote:
> > > On Fri, Oct 2, 2026 at 8:59 AM Andy Shevchenko

...

> > > The entire name of the thing feels like debugfs-footgun
> > > territory for example.
> >
> > Yeah, I can't find neither a thing on LWN.net nor in the in-tree documentation.
> > The only useful piece of information is (in kernel-doc of struct bus_type):
> >
> >   driver_override
> >     Set to true if this bus supports the driver_override mechanism, which
> >     allows userspace to force a specific driver to bind to a device via a sysfs
> >     attribute.
> 
> This whole thing is weird, but OK.
> 
> Since we have a ton of drivers depending on match data we either
> have to patch them all to bail out if match data is NULL (like this
> patch does) or, which is equivalent, opt out of driver_override
> that much is certain.
> 
> What I don't get is what this is intended for. What is the use case?
> The commit says this is for VFIO. Shouldn't it be opt-in and turned
> on only for VFIO then?
> 
> Kim Phillips is listed as contract for the platform bus driver_override
> so let's ask! Kim: what is this for?
> 
> All of the device tree drivers use the platform bus, what's yhe
> Unique Selling Point of this for our devices?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Danilo Krummrich]

On Sun Oct 4, 2026 at 10:15 AM CEST, Andy Shevchenko wrote:
> +Cc: Danilo
> (as you were involved in cleaning this up in the past and being co-maintainer
>  of driver core)
>
> On Sun, Oct 04, 2026 at 01:06:07AM +0200, Linus Walleij wrote:
>> On Fri, Oct 2, 2026 at 12:19 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>> > On Fri, Oct 02, 2026 at 11:52:41AM +0200, Linus Walleij wrote:
>> > > On Fri, Oct 2, 2026 at 8:59 AM Andy Shevchenko
>
> ...
>
>> > > The entire name of the thing feels like debugfs-footgun
>> > > territory for example.
>> >
>> > Yeah, I can't find neither a thing on LWN.net nor in the in-tree documentation.
>> > The only useful piece of information is (in kernel-doc of struct bus_type):
>> >
>> >   driver_override
>> >     Set to true if this bus supports the driver_override mechanism, which
>> >     allows userspace to force a specific driver to bind to a device via a sysfs
>> >     attribute.
>> 
>> This whole thing is weird, but OK.
>> 
>> Since we have a ton of drivers depending on match data we either
>> have to patch them all to bail out if match data is NULL (like this
>> patch does) or, which is equivalent, opt out of driver_override
>> that much is certain.
>> 
>> What I don't get is what this is intended for. What is the use case?
>> The commit says this is for VFIO. Shouldn't it be opt-in and turned
>> on only for VFIO then?

I think VFIO could use a different (less generic) mechanism that is more
integrated with the corresponding bus to e.g. allow userspace to decide to get a
PF bound to a VFIO driver for passthrough.

The existing driver_override is convinient for this case, but ideally we want to
express that a driver can only be bound to the corresponding host and VFIO
drivers.

I think the situation for SPI is pretty similar.

Unfortunately, it is a uAPI already, so we can't really get rid of it.

But I agree that we should be more defensive about this and make it a driver
opt-in. Uwe already prepared a patch for this [1].

[1] https://lore.kernel.org/driver-core/0f7446324f6a0c8f0153d6532d92a6eeecd6a308.1790612298.git.u.kleine-koenig@baylibre.com/

Re: [PATCH] pinctrl: mcp23s08: reject devices without match data

[Linus Walleij]

On Wed, Oct 7, 2026 at 1:07 PM Danilo Krummrich <dakr@kernel.org> wrote:

> >> What I don't get is what this is intended for. What is the use case?
> >> The commit says this is for VFIO. Shouldn't it be opt-in and turned
> >> on only for VFIO then?
>
> I think VFIO could use a different (less generic) mechanism that is more
> integrated with the corresponding bus to e.g. allow userspace to decide to get a
> PF bound to a VFIO driver for passthrough.
>
> The existing driver_override is convinient for this case, but ideally we want to
> express that a driver can only be bound to the corresponding host and VFIO
> drivers.
>
> I think the situation for SPI is pretty similar.
>
> Unfortunately, it is a uAPI already, so we can't really get rid of it.

No, but we *could* make the whole thing a Kconfig option and have it
default n, and selected only for VFIO.

I have a strong urge to cook a patch like that...

> But I agree that we should be more defensive about this and make it a driver
> opt-in. Uwe already prepared a patch for this [1].
>
> [1] https://lore.kernel.org/driver-core/0f7446324f6a0c8f0153d6532d92a6eeecd6a308.1790612298.git.u.kleine-koenig@baylibre.com/

I like what I see.

Yours,
Linus Walleij