gpio: cdev: sanitize the label before requesting the interrupt
Subsystems (3)
PATCH v3 1/1 gpio: cdev: sanitize the label before requesting the interrupt
From: Bartosz Golaszewski <[email protected]>
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 ":".
---
v2 -> v3:
- fix a memory leak in error path
v1 -> v2:
- use ':' as the delimiter instead of '-'
- return -ENOMEM if creating the label fails
drivers/gpio/gpiolib-cdev.c | 38 +++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-)
drivers/gpio/gpiolib-cdev.c +32 -6
@@ -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;
Signature · 1 line
2.40.1
Quoted 6 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
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.
Quoted 8 lines
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 ":".
Looks good to me.
Quoted 12 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
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.
Quoted 6 lines
Let's replace all "/" with ":".
Good work on this one!
Yours,
Linus Walleij
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.
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
Quoted 54 lines · Naresh Kamboju
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 commitb34490879baa847d16fc529c8ea6e6d34f004b38upstream.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_settingsTest 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_eventsSteps to reproduce:
-----
https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/tests/2eUlyN8HN4R1u0RyLwN6hx7IV0I/reproducerLinks:
- 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/
Quoted 3 lines
--
Linaro LKFT
https://lkft.linaro.org
Hi!
Yes, I confirm the issue, I will have a fix ready soon.
Thanks!
Bart