Re: [PATCH v2] gpio: cdev: fix kernel stack leak to user-space in error path

[Bartosz Golaszewski]

On Mon, 21 Sep 2026 15:35:05 +0200, Kent Gibson <warthog618@xxxxxxxxx> said:
> On Mon, Sep 21, 2026 at 02:56:58PM +0200, Bartosz Golaszewski wrote:
>> If we fail to acquire the GPIO chip guard in gpio_desc_to_lineinfo(), we
>> return immediately before zeroing the info struct we'll end up passing
>> to the user-space later in lineinfo_get_v1(). This may leak the kernel
>> stack contents. Make gpio_desc_to_lineinfo() return int so that the
>> -ENODEV returned on failure to acquire the guard can be propagated to
>> the callers.
>>
>> While not strictly necessary: move the memset() before trying to acquire
>> the SRCU read lock too for good measure.
>>
>> Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
>> Closes: https://sashiko.dev/#/patchset/20260912123529.7951-1-tzungbi%40kernel.org?part=3
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
>> ---
>> Changes in v2:
>> - Make gpio_desc_to_lineinfo() return an integer return value so that
>>   the error code returned in case of a failure to acquire the guard can
>>   be propagated down the stack
>> - Link to v1: https://patch.msgid.link/20260917-gpio-cdev-stack-leak-fixes-v1-1-2bb52879e49c@xxxxxxxxxxxxxxxx
>
>> @@ -2521,7 +2533,14 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
>>
>>  	ctx->chg.event_type = action;
>>  	ctx->chg.timestamp_ns = ktime_get_ns();
>> -	gpio_desc_to_lineinfo(desc, &ctx->chg.info, true);
>> +
>> +	ret = gpio_desc_to_lineinfo(desc, &ctx->chg.info, true);
>> +	if (ret) {
>> +		fput(fp);
>> +		pr_err("GPIO controller that generated the line info notification was removed\n");
>> +		return NOTIFY_DONE;
>> +	}
>> +
>
> Does resolving a race warrant an error?
>

I guess not, we can just not do anything in this case.

> Also, compared to the kzalloc_obj() error path earlier in the function, here
> you flip the order of the fput() and pr_err(). Is that significant?
>

No, if anything this version is more correct as we drop the no-longer-needed
resource before printing a log. But with the above it's irrelevant anyway.

Bart

> Other than that, looks good to me.
>
> Cheers,
> Kent.
>