Re: [PATCH v4 2/2] spi: Add NULL check for spi_get_device_id() in spi_get_device_match_data()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] spi_get_device_id() unconditionally dereferences a potentially NULL id_table pointer
- [High] Direct callers of spi_get_device_id() unconditionally dereference its result
- [High] Callers of the patched spi_get_device_match_data() unconditionally dereference its NULL return value
--

commit 7e155f379f89eda6ab91795ac56410fd02c722be
Author: guoqi0226 <guoqi0226@163.com>

spi: Add NULL check for spi_get_device_id() in spi_get_device_match_data()

Prevent NULL pointer dereference when spi_get_device_id() returns NULL,
which can happen when using driver_override without matching SPI ID entry.

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 104279858f567..079ac01a2229d 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -355,12 +355,16 @@ EXPORT_SYMBOL_GPL(spi_get_device_id);
>  const void *spi_get_device_match_data(const struct spi_device *sdev)
>  {
>  	const void *match;
> +	const struct spi_device_id *id;
>  
>  	match = device_get_match_data(&sdev->dev);
>  	if (match)
>  		return match;
>  
> -	return (const void *)spi_get_device_id(sdev)->driver_data;
> +	id = spi_get_device_id(sdev);

[Severity: High]
This is a pre-existing issue, but can spi_get_device_id() itself crash
if sdrv->id_table is NULL?

If a spi driver only provides a device tree or acpi match table and lacks
an id_table, sdrv->id_table will evaluate to NULL. When spi_get_device_id()
is called, it passes this NULL pointer to spi_match_id():

drivers/spi/spi.c:spi_get_device_id() {
    ...
    return spi_match_id(sdrv->id_table, sdev->modalias);
}

spi_match_id() then unconditionally evaluates id->name[0] in its while loop:

drivers/spi/spi.c:spi_match_id() {
    ...
    while (id->name[0]) {
    ...
}

Would this result in a NULL pointer dereference when driver_override is used?

[Severity: High]
This is also a pre-existing issue, but do many direct callers of
spi_get_device_id() unconditionally dereference its result?

For example, in drivers/iio/adc/max1118.c:max1118_probe():

    indio_dev->name = spi_get_device_id(spi)->name;

If driver_override is used to force a bind to a device whose modalias
does not match the driver's id_table, spi_get_device_id() can return NULL.
Does this mean individual spi drivers might immediately crash if they
don't check the return value?

> +	if (!id)
> +		return NULL;

[Severity: High]
This isn't a bug introduced by this patch, but will returning NULL here just
shift the crash to callers that don't check the return value?

For example, in drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_probe():

    smd->mode = spi_get_device_match_data(spi);
    ...
    smd->pitch = (SHARP_ADDR_PERIOD + smd->mode->hdisplay + ...) / 8;

If spi_get_device_match_data() returns NULL when driver_override is used,
will callers unconditionally dereference it and still cause a kernel panic?

> +	return (const void *)id->driver_data;
>  }
>  EXPORT_SYMBOL_GPL(spi_get_device_match_data);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616103018.105612-1-guoqi0226@163.com?part=2