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

[Kent Gibson <warthog618@gmail.com>]

On Tue, Sep 22, 2026 at 02:33:35AM -0700, Bartosz Golaszewski wrote:
> On Tue, 22 Sep 2026 11:09:55 +0200, Kent Gibson <warthog618@gmail.com> said:
> > On Tue, Sep 22, 2026 at 10:52:28AM +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@vger.kernel.org
> >> Reported-by: Sashiko <sashiko-bot@kernel.org>
> >> Closes: https://sashiko.dev/#/patchset/20260912123529.7951-1-tzungbi%40kernel.org?part=3
> >> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> >> ---
> >> Changes in v3:
> >> - Don't emit an error log message from the line info notifier if we fail
> >>   to acquire the chip guard
> >> - Link to v2: https://patch.msgid.link/20260921-gpio-cdev-stack-leak-fixes-v2-1-3307c4320352@oss.qualcomm.com
> >>
> >> 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@oss.qualcomm.com
> >> ---
> >>  drivers/gpio/gpiolib-cdev.c | 32 +++++++++++++++++++++++++-------
> >>  1 file changed, 25 insertions(+), 7 deletions(-)
> >>
> >
> >> @@ -2491,6 +2502,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
> >>  	struct lineinfo_changed_ctx *ctx;
> >>  	struct gpio_desc *desc = data;
> >>  	struct file *fp;
> >> +	int ret;
> >>
> >>  	if (!test_bit(gpiod_hwgpio(desc), cdev->watched_lines))
> >>  		return NOTIFY_DONE;
> >> @@ -2521,7 +2533,13 @@ 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);
> >> +		return NOTIFY_DONE;
> >> +	}
> >> +
> >
> > Drop the ret and just test the return directly?
> >
> > Sorry - should've picked that up in the previous version.
> >
> 
> I actually prefer the assignment, I find it more readable.
> 

I'm the reverse, but I can live with it.

Reviewed-by: Kent Gibson <warthog618@gmail.com>

Cheers,
Kent.