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

[jiale yao]

At 2026-09-28 15:32:52, "Biju Das" <biju.das.jz@xxxxxxxxxxxxxx> wrote:
>
>
>> -----Original Message-----
>> From: jiale yao <19888972804@xxxxxxx>
>> Sent: 28 September 2026 08:25
>> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>> Cc: Linus Walleij <linusw@xxxxxxxxxx>; Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>; linux-
>> gpio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>> Subject: Re:RE: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without match data
>> 
>> [You don't often get email from 19888972804@xxxxxxx. Learn why this is important at
>> https://aka.ms/LearnAboutSenderIdentification ]
>> 
>> At 2026-09-28 14:03:46, "Biju Das" <biju.das.jz@xxxxxxxxxxxxxx> wrote:
>> >Hi jiale yao,
>> >
>> >> -----Original Message-----
>> >> From: jiale yao <19888972804@xxxxxxx>
>> >> Sent: 26 September 2026 11:24
>> >> Subject: Re:RE: [PATCH] pinctrl: mcp23s08: reject devices without
>> >> match data
>> >>
>> >> [You don't often get email from 19888972804@xxxxxxx. Learn why this
>> >> is important at https://aka.ms/LearnAboutSenderIdentification ]
>> >>
>> >> Hi Biju,
>> >>
>> >> At 2026-09-26 18:07:14, "Biju Das" <biju.das.jz@xxxxxxxxxxxxxx> wrote:
>> >> >Hi,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Jiale Yao <yaojiale02@xxxxxxx>
>> >> >> Sent: 25 September 2026 14:38
>> >> >> Subject: [PATCH] pinctrl: mcp23s08: reject devices without match
>> >> >> data
>> >> >>
>> >> >> A device bound through driver_override need not match an entry in
>> >> >> the driver tables. In that case
>> >> >> spi_get_device_match_data() returns NULL, but mcp23s08_probe()
>> >> >> later passes the result to the regmap setup and dereferences it while setting up each device.
>> >> >
>> >> >I believe this is an invalid use case as the user trying
>> >> >driver_override and the probe returns error. Am I missing anything here?
>> >>
>> >> Yes, my commit message was not clear enough. This patch is intended
>> >> to fix a regression introduced by commit 2e44555b05c0, rather than to make an unmatched device probe
>> successfully through driver_override.
>> >>
>> >> Before that commit, missing match data effectively resulted in type
>> >> 0, which fell through to the default switch case and returned -EINVAL.
>> >> Commit 2e44555b05c0 replaced the type value with an info pointer, but
>> >> the SPI path missed the corresponding NULL check before dereferencing
>> >> info->type. The I2C path preserved the previous error handling with
>> >> info->an
>> >> explicit NULL check after the same conversion.
>> >
>> >you are testing the driver with driver_override and your patch makes
>> >driver Probe failure. I do not understand the test case you are trying
>> >to achieve with driver_override.
>> >
>> >Can you please share some logs with driver_override that you planned to test?
>> >
>> >Cheers,
>> >Biju
>> >
>> >>
>> >> Consequently, an unmatched device bound through driver_override used
>> >> to fail cleanly. It can now cause a NULL pointer dereference when a
>> >> valid spi-present-mask property is present. This patch restores the previous failure behavior.
>> >>
>> >> >
>> >> >Can you please prepare a patch that makes the driver probe success
>> >> >by using driver_override Feature? Also please share some logs to see
>> >> >how it works in real device
>> >> >
>> >> >Cheers,
>> >> >Biju
>> 
>> 
>> The test is NOT intended to make an unmatched device probe successfully.
>> driver_override permits an explicit probe attempt; it does not guarantee that probe must succeed.
>> The driver must still reject unsupported devices safely. The bug is that it currently crashes instead of
>> returning an error.
>> 
>> 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 reports:
>
>This is original issue.
>You want to now add driver_override feature.
>
>What is the log you get with your patch, probe failure?
>
>Can't you make a patch for making probe working with driver_override feature
>that makes the device functional with driver_override?

I think we are discussing two different expectations:

- driver_override only gives the named driver an opportunity to bind, as
    documented in Documentation/ABI/testing/sysfs-bus-platform. It does
    not guarantee that probe succeeds.
- Without match data, this driver cannot determine whether the device is
    an MCP23S08, MCP23S17 or MCP23S18. Probe must therefore fail.
- Before commit 2e44555b05c0, this path failed cleanly with -EINVAL.
    After that commit, it dereferences NULL.
- This patch only restores the safe failure behavior.

With the patch applied, the result is:

    [    2.319112] mcp23s08 spi0.0: probe with driver mcp23s08 failed with error -61

This is the expected result. As far as I understand, an unsuccessful probe should
return an error, not cause a kernel memory access fault.

I am a little confused by this point. Are you suggesting that, if driver_override is
used for an unsupported device, the kernel should crash rather than have the
driver's probe fail cleanly?

>
>> 
>> KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
>> pc : mcp23s08_probe+0x2f8/0x944
>> lr : mcp23s08_probe+0x190/0x944
>> Call trace:
>>  mcp23s08_probe+0x2f8/0x944 (P)
>>  spi_probe+0x1c0/0x2d8
>>  really_probe+0x17c/0x5b8
>>  __driver_probe_device+0x28c/0x350
>>  device_driver_attach+0xa0/0x18c
>>  bind_store+0xc4/0x13c
>>  drv_attr_store+0x60/0x9c
>>  sysfs_kf_write+0x170/0x1e8
>>  kernfs_fop_write_iter+0x298/0x404
>>  vfs_write+0x648/0x8cc
>>  ksys_write+0xf0/0x1e0
>>  __arm64_sys_write+0x70/0xa0
>>  invoke_syscall+0x70/0x24c
>> 
>> 
>> spi_get_device_match_data() returns NULL, and mcp23s08_spi_regmap_init() later dereferences info->type at
>> offset 16.
>> 
>> The patch only guarantees safety; it does not attempt to support unmatched hardware through
>> driver_override.
>
>If you don't want driver_override non functional
>What is the point of this patch, simply want to add some
>Code space in linux kernel?
>
>Cheers,
>Biju