[PATCH] gpioib: do not free unrequested descriptors

[PATCH] gpioib: do not free unrequested descriptors

[Timur Tabi]

If the main loop in linehandle_create() encounters an error, it
unwinds completely by freeing all previously requested GPIO
descriptors.  However, if the error occurs in the beginning of
the loop before that GPIO is requested, then the exit code
attempts to free a null descriptor.  If extrachecks is enabled,
gpiod_free() triggers a WARN_ON.

Instead, keep a separate count of legitimate GPIOs so that only
those are freed.

Cc: stable@vger.kernel.org
Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines")
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/gpio/gpiolib.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 43aeb07343ec..d07771797707 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -497,7 +497,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	struct gpiohandle_request handlereq;
 	struct linehandle_state *lh;
 	struct file *file;
-	int fd, i, ret;
+	int fd, i, count = 0, ret;
 	u32 lflags;
 
 	if (copy_from_user(&handlereq, ip, sizeof(handlereq)))
@@ -558,6 +558,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 		if (ret)
 			goto out_free_descs;
 		lh->descs[i] = desc;
+		count = i;
 
 		if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW)
 			set_bit(FLAG_ACTIVE_LOW, &desc->flags);
@@ -628,7 +629,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 out_put_unused_fd:
 	put_unused_fd(fd);
 out_free_descs:
-	for (; i >= 0; i--)
+	for (i = 0; i < count; i++)
 		gpiod_free(lh->descs[i]);
 	kfree(lh->label);
 out_free_lh:
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] gpioib: do not free unrequested descriptors

[Timur Tabi]

On 3/29/18 1:29 PM, Timur Tabi wrote:
> If the main loop in linehandle_create() encounters an error, it
> unwinds completely by freeing all previously requested GPIO
> descriptors.  However, if the error occurs in the beginning of
> the loop before that GPIO is requested, then the exit code
> attempts to free a null descriptor.  If extrachecks is enabled,
> gpiod_free() triggers a WARN_ON.
> 
> Instead, keep a separate count of legitimate GPIOs so that only
> those are freed.

Linus, this is an important fix that's needed for sparse GPIO support. 
Any chance that it can make 4.17?

Also, my other patchset for qdf2xxx support has been reviewed by Bjorn 
and Stephen.  Can you add that to 4.17 also?

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH] gpioib: do not free unrequested descriptors

[Linus Walleij]

On Thu, Mar 29, 2018 at 8:29 PM, Timur Tabi <timur@codeaurora.org> wrote:

> If the main loop in linehandle_create() encounters an error, it
> unwinds completely by freeing all previously requested GPIO
> descriptors.  However, if the error occurs in the beginning of
> the loop before that GPIO is requested, then the exit code
> attempts to free a null descriptor.  If extrachecks is enabled,
> gpiod_free() triggers a WARN_ON.
>
> Instead, keep a separate count of legitimate GPIOs so that only
> those are freed.
>
> Cc: stable@vger.kernel.org
> Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines")
> Signed-off-by: Timur Tabi <timur@codeaurora.org>

Patch applied for fixes.

Bartosz, can you have a quick look at this? Did you run into the
problem at any point?

Yours,
Linus Walleij

Re: [PATCH] gpioib: do not free unrequested descriptors

[Bartosz Golaszewski]

2018-04-26 11:17 GMT+02:00 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Mar 29, 2018 at 8:29 PM, Timur Tabi <timur@codeaurora.org> wrote:
>
>> If the main loop in linehandle_create() encounters an error, it
>> unwinds completely by freeing all previously requested GPIO
>> descriptors.  However, if the error occurs in the beginning of
>> the loop before that GPIO is requested, then the exit code
>> attempts to free a null descriptor.  If extrachecks is enabled,
>> gpiod_free() triggers a WARN_ON.
>>
>> Instead, keep a separate count of legitimate GPIOs so that only
>> those are freed.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines")
>> Signed-off-by: Timur Tabi <timur@codeaurora.org>
>
> Patch applied for fixes.
>
> Bartosz, can you have a quick look at this? Did you run into the
> problem at any point?
>

I have never seen this issue, but the patch looks correct to me.

Thanks,
Bartosz

Re: [PATCH] gpioib: do not free unrequested descriptors

[Timur Tabi]

On 04/26/2018 11:44 AM, Bartosz Golaszewski wrote:
>>
>> Bartosz, can you have a quick look at this? Did you run into the
>> problem at any point?
>>
> I have never seen this issue, but the patch looks correct to me.

The issue can only occur if you have sparse GPIOs, and you tried to 
request one that doesn't exist, which probably never happened prior to 4.17.

Thanks for the review.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.