diff mbox

[1/3] gpioib: do not free unrequested descriptors

Message ID 1524696207-5156-2-git-send-email-timur@codeaurora.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Timur Tabi April 25, 2018, 10:43 p.m. UTC
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(-)

Comments

Bjorn Andersson April 25, 2018, 10:58 p.m. UTC | #1
On Wed 25 Apr 15:43 PDT 2018, 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.
> 
> Cc: stable@vger.kernel.org
> Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines")
> Signed-off-by: Timur Tabi <timur@codeaurora.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  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.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij April 26, 2018, 9:27 a.m. UTC | #2
On Thu, Apr 26, 2018 at 12:43 AM, 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>

The earlier version of this is already queued for fixes,
I'll add Bjorn's Review tag.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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: