Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver

Rasmus Villemoes
2021-08-19T12:09:38Z

On 19/08/2021 13.46, Wolfgang Denk wrote:
> Dear Rasmus,
>
> again: error handling.
>
> In message <20210819095706.3585923-11-rasmus.villemoes@prevas.dk> you wrote:
>>
>> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
>> new file mode 100644
>> index 0000000000..982a66b3f9
>> --- /dev/null
>> +++ b/drivers/watchdog/gpio_wdt.c
>> @@ -0,0 +1,68 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#include <dm.h>
>> +#include <dm/device_compat.h>
>> +#include <wdt.h>
>> +#include <asm/gpio.h>
>> +
>> +struct gpio_wdt_priv {
>> + struct gpio_desc gpio;
>> + bool always_running;
>> + int state;
>> +};
>> +
>> +static int gpio_wdt_reset(struct udevice *dev)
>> +{
>> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
>> +
>> + priv->state = !priv->state;
>
> Potential NULL pointer dereference.

No, no and no. If allocation of the (driver or uclass) private data
fails, the device probe would have failed, so this code can never get
called with such a struct udevice.

Perhaps try doing a

git grep -10 -E 'dev_get(_uclass)?_priv'

and see how many cases you can find where that is followed by a NULL check?

Rasmus