Re: [PATCH 2/3] staging: iio: accel: remove adis16203, merged into mainline adis16201 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The IIO device name is incorrectly set to the combined SPI driver name ('adis16201_adis16203'), breaking userspace ABI.
- [High] Missing check for spi_get_device_match_data() returning NULL leads to a crash on non-OF probes.
--

commit 4a7d80a7646bf1890c13a1b5b3c99c166b2c3898
Author: Shehryar Ahmad <shehryar.amd@gmail.com>

iio: accel: adis16201: merge adis16203 support from staging
    
This commit merges the ADIS16203 inclinometer driver from staging into the
mainline ADIS16201 driver. It handles chip differences using a differ_info
struct passed via of_device_id match data and keeps the channel arrays separate.
The SPI driver name is also updated to reflect the combined driver support.

> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> index 5127e58eebc7d..293655fb00531 100644
> --- a/drivers/iio/accel/adis16201.c
> +++ b/drivers/iio/accel/adis16201.c

[ ... ]

> @@ -248,56 +301,62 @@ static const struct adis_data adis16201_data = {
>  	.timeouts = &adis16201_timeouts,
>  
>  	.status_error_msgs = adis16201_status_error_msgs,
> -	.status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
>  };
>  
>  static int adis16201_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
> -	struct adis *st;
> +	struct adis16201_state *st;
> +	struct adis_data data = adis16201_data;
>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(struct adis16201_state));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> +	st->info = spi_get_device_match_data(spi);
>  
>  	indio_dev->name = spi->dev.driver->name;

[Severity: Critical]
Does this code assign the combined SPI driver name "adis16201_adis16203" to
the IIO device name? The IIO name attribute is relied upon by applications
and libraries (like libiio) to identify the specific hardware part.

Since the driver name was changed from "adis16201" to "adis16201_adis16203",
the sysfs name attribute now statically exposes "adis16201_adis16203" for all
instantiated devices regardless of the actual hardware, breaking the userspace
ABI.

>  	indio_dev->info = &adis16201_info;
> -
> -	indio_dev->channels = adis16201_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
> +	indio_dev->channels = st->info->arr_chans;

[Severity: High]
Can this lead to a NULL pointer dereference during non-OF probes? If the device
is instantiated via legacy SPI board info or sysfs new_device with the driver
name "adis16201_adis16203", the SPI core matches it and calls adis16201_probe()
in drivers/iio/accel/adis16201.c.

Because it's not an OF/ACPI match, spi_get_device_match_data() falls back to
checking the driver's id_table. Since the id_table is missing from
adis16201_driver, spi_get_device_match_data() returns NULL, and dereferencing
it via st->info->arr_chans causes a kernel crash.

> +	indio_dev->num_channels = st->info->num_chans;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	ret = adis_init(st, indio_dev, spi, &adis16201_data);
> +	data.status_error_mask = st->info->diag_stat_mask;
> +	ret = adis_init(&st->adis, indio_dev, spi, &data);

[ ... ]

> @@ -318,10 +372,11 @@ static int adis16201_probe(struct spi_device *spi)
>  
>  static struct spi_driver adis16201_driver = {
>  	.driver = {
> -		.name = "adis16201",
> +		.name = "adis16201_adis16203",
> +		.of_match_table = adis16201_of_match,
>  	},
>  	.probe = adis16201_probe,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831184201.34416-1-shehryar.amd@gmail.com?part=2