| Message ID | 1522348152-15024-1-git-send-email-timur@codeaurora.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpioib: do not free unrequested descriptors | expand |
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 04/15/2018 10:20 PM, Timur Tabi wrote: > > 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? Linus, I don't want to be a pest, but I noticed you updated your git.kernel.org repo and none of my patches are there. I really would like to see my patches, which have been reviewed, in 4.17.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 20, 2018 at 10:36 PM, Timur Tabi <timur@codeaurora.org> wrote: > On 04/15/2018 10:20 PM, Timur Tabi wrote: >> >> >> 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? > > Linus, I don't want to be a pest, but I noticed you updated your > git.kernel.org repo and none of my patches are there. I really would like > to see my patches, which have been reviewed, in 4.17. I am sorry that the process is not always as expedient as one could wish. The v4.17-rc1 was tagged on april 15 and at that point we rarely add new functionality. I usually need to have review finished some ~2 weeks before the merge window, with some exceptions where things are smooth or if stuff dragged out. I understand that sometimes the kernel "process" can be perceived as bureaucratic, permeated by the bureaucratic incentive to act slowly and impersonal, such as the planets, or the plants. I am honestly trying to do my best. Yours. Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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:
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(-)