[PATCH] drivers: spi: fix deref ater null.might in spi-uclass.c

[Simon Glass]

Hi,

On Fri, 16 May 2025 at 14:51, <ant.v.moryakov at gmail.com> wrote:
>
> From: Anton Moryakov <ant.v.moryakov at gmail.com>
>
> The static analyzer (Svace) reported
> After having been compared to a NULL value at spi-uclass.c:465,
> pointer 'dev' is passed as 1st parameter in call to function 'dev_get_flags'
> at spi-uclass.c:469, where it is dereferenced at device.h:240.
>
> Correct explained:
> 1. Added dev && !device_active(dev) check before calling device_active()
> 2. Added explicit if (!dev) check with ret = -ENODEV setting
> 3. Protected logging in error block with if(dev) check
>
> Signed-off-by: Anton Moryakov <ant.v.moryakov at gmail.com>
> ---
>  drivers/spi/spi-uclass.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> index d6049753740..52b79223f96 100644
> --- a/drivers/spi/spi-uclass.c
> +++ b/drivers/spi/spi-uclass.c
> @@ -345,7 +345,7 @@ int spi_get_bus_and_cs(int busnum, int cs, struct udevice **busp,
>                 return ret;
>         }
>
> -       if (!device_active(dev)) {
> +       if (dev && !device_active(dev)) {
>                 struct spi_slave *slave;
>
>                 ret = device_probe(dev);
> @@ -355,6 +355,11 @@ int spi_get_bus_and_cs(int busnum, int cs, struct udevice **busp,
>                 slave->dev = dev;
>         }
>
> +       if (!dev) {
> +               ret = -ENODEV;
> +               goto err;
> +       }
> +
>         slave = dev_get_parent_priv(dev);
>         bus_data = dev_get_uclass_priv(bus);
>
> @@ -373,8 +378,11 @@ int spi_get_bus_and_cs(int busnum, int cs, struct udevice **busp,
>         return 0;
>
>  err:
> -       log_debug("%s: Error path, device '%s'\n", __func__, dev->name);
> -
> +       if(dev)
> +               log_debug("%s: Error path, device '%s'\n", __func__, dev->name);
> +       else
> +               log_debug("%s: Error path, NULL device\n", __func__);
> +
>         return ret;
>  }
>
> --
> 2.30.2
>

You are not allowed to pass NULL as dev to driver model functions. So
the bug here is in the SPI code.

There is a code-size cost to checking arguments and we tend to be
reluctant to pay it in U-Boot, where code size is often critical.

We do have assert() but it is fairly intrusive and people seldom
enable it. If you have a better way, please suggest it!

Regards,
Simon