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:
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?
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
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
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.
linux-stable-mirror@lists.linaro.org