| Message ID | 20240325090242.14281-1-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] gpio: cdev: sanitize the label before requesting the interrupt | expand |
On Mon, Mar 25, 2024 at 10:02 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > When an interrupt is requested, a procfs directory is created under > "/proc/irq/<irqnum>/<label>" where <label> is the string passed to one of > the request_irq() variants. > > What follows is that the string must not contain the "/" character or > the procfs mkdir operation will fail. We don't have such constraints for > GPIO consumer labels which are used verbatim as interrupt labels for > GPIO irqs. We must therefore sanitize the consumer string before > requesting the interrupt. > > Let's replace all "/" with ":". > > Cc: stable@vger.kernel.org > Reported-by: Stefan Wahren <wahrenst@gmx.net> > Closes: https://lore.kernel.org/linux-gpio/39fe95cb-aa83-4b8b-8cab-63947a726754@gmx.net/ > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Good work on this one! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Apr 2, 2024 at 11:35 AM Naresh Kamboju <naresh.kamboju@linaro.org> wrote: > > Results from Linaro’s test farm. > Regressions on arm64, arm, x86_64, and i386 with libgpiod tests. > > libgpiod test regressions noticed on Linux stable-rc 6.8, 6.7 and 6.6 > and Linux next and mainline master v6.9-rc2. > > Anders bisected and found this first bad commit, > gpio: cdev: sanitize the label before requesting the interrupt > commit b34490879baa847d16fc529c8ea6e6d34f004b38 upstream. > > Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > > LKFT is running libgpiod test suite version > v2.0.1-0-gae275c3 (and also tested v2.1) > > libgpiod > - _gpiod_edge-event_edge_event_wait_timeout > - _gpiod_edge-event_event_copy > - _gpiod_edge-event_null_buffer > - _gpiod_edge-event_read_both_events > - _gpiod_edge-event_read_both_events_blocking > - _gpiod_edge-event_read_falling_edge_event > - _gpiod_edge-event_read_rising_edge_event > - _gpiod_edge-event_read_rising_edge_event_polled > - _gpiod_edge-event_reading_more_events_than_the_queue_contains_doesnt_block > - _gpiod_edge-event_seqno > - _gpiod_line-info_edge_detection_settings > > Test log: > ------- > ok 16 /gpiod/edge-event/edge_event_buffer_max_capacity > ** > gpiod-test:ERROR:tests-edge-event.c:52:_gpiod_test_func_edge_event_wait_timeout: '_request' should not be NULL > # gpiod-test:ERROR:tests-edge-event.c:52:_gpiod_test_func_edge_event_wait_timeout: '_request' should not be NULL > not ok 17 /gpiod/edge-event/edge_event_wait_timeout > ok 18 /gpiod/edge-event/cannot_request_lines_in_output_mode_with_edge_detection > ** > gpiod-test:ERROR:tests-edge-event.c:125:_gpiod_test_func_read_both_events: '_request' should not be NULL > # gpiod-test:ERROR:tests-edge-event.c:125:_gpiod_test_func_read_both_events: '_request' should not be NULL > not ok 19 /gpiod/edge-event/read_both_events > > Steps to reproduce: > ----- > https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/tests/2eUlyN8HN4R1u0RyLwN6hx7IV0I/reproducer > > Links: > - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240402/testrun/23265184/suite/libgpiod/tests/ > - https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v6.9-rc2/testrun/23244617/suite/libgpiod/tests/ > - https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/tests/2eUlyN8HN4R1u0RyLwN6hx7IV0I > - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.8.y/build/v6.8.2-400-gbffeaccf18b5/testrun/23252337/suite/libgpiod/tests/ > - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.7.y/build/v6.7.11-433-gb15156435f06/testrun/23252698/suite/libgpiod/tests/ > - https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-6.6.y/build/v6.6.23-397-g75a2533b74d0/testrun/23254910/suite/libgpiod/tests/ > > -- > Linaro LKFT > https://lkft.linaro.org Hi! Yes, I confirm the issue, I will have a fix ready soon. Thanks! Bart
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index f384fa278764..fa9635610251 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -1083,10 +1083,20 @@ static u32 gpio_v2_line_config_debounce_period(struct gpio_v2_line_config *lc, return 0; } +static inline char *make_irq_label(const char *orig) +{ + return kstrdup_and_replace(orig, '/', ':', GFP_KERNEL); +} + +static inline void free_irq_label(const char *label) +{ + kfree(label); +} + static void edge_detector_stop(struct line *line) { if (line->irq) { - free_irq(line->irq, line); + free_irq_label(free_irq(line->irq, line)); line->irq = 0; } @@ -1110,6 +1120,7 @@ static int edge_detector_setup(struct line *line, unsigned long irqflags = 0; u64 eflags; int irq, ret; + char *label; eflags = edflags & GPIO_V2_LINE_EDGE_FLAGS; if (eflags && !kfifo_initialized(&line->req->events)) { @@ -1146,11 +1157,17 @@ static int edge_detector_setup(struct line *line, IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING; irqflags |= IRQF_ONESHOT; + label = make_irq_label(line->req->label); + if (!label) + return -ENOMEM; + /* Request a thread to read the events */ ret = request_threaded_irq(irq, edge_irq_handler, edge_irq_thread, - irqflags, line->req->label, line); - if (ret) + irqflags, label, line); + if (ret) { + free_irq_label(label); return ret; + } line->irq = irq; return 0; @@ -1973,7 +1990,7 @@ static void lineevent_free(struct lineevent_state *le) blocking_notifier_chain_unregister(&le->gdev->device_notifier, &le->device_unregistered_nb); if (le->irq) - free_irq(le->irq, le); + free_irq_label(free_irq(le->irq, le)); if (le->desc) gpiod_free(le->desc); kfree(le->label); @@ -2114,6 +2131,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip) int fd; int ret; int irq, irqflags = 0; + char *label; if (copy_from_user(&eventreq, ip, sizeof(eventreq))) return -EFAULT; @@ -2198,15 +2216,23 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip) if (ret) goto out_free_le; + label = make_irq_label(le->label); + if (!label) { + ret = -ENOMEM; + goto out_free_le; + } + /* Request a thread to read the events */ ret = request_threaded_irq(irq, lineevent_irq_handler, lineevent_irq_thread, irqflags, - le->label, + label, le); - if (ret) + if (ret) { + free_irq_label(label); goto out_free_le; + } le->irq = irq;