gpio: cdev: label sanitization fixes

From: Bartosz Golaszewski <[email protected]>

We need to take into account that a line's consumer label may be NULL and not try to kstrdup() it in that case but rather pass the NULL pointer up the stack to the interrupt request function.

To that end: let make_irq_label() return NULL as a valid return value and use ERR_PTR() instead to signal an allocation failure to callers.

Cc [email protected]

Fixes b34490879baa ("gpio: cdev: sanitize the label before requesting the interrupt")

Reported-by Linux Kernel Functional Testing <[email protected]>

Closes https://lore.kernel.org/lkml/[email protected]/ open here

Signed-off-by Bartosz Golaszewski <[email protected]>

---

From: Kent Gibson <[email protected]>

When adding sanitization of the label, the path through edge_detector_setup() that leads to debounce_setup() was overlooked. A request taking this path does not allocate a new label and the request label is freed twice when the request is released, resulting in memory corruption.

Add label sanitization to debounce_setup().

drivers/gpio/gpiolib-cdev.c +8 -0

+static inline char *make_irq_label(const char *orig)
+{
+	char *new;
+
+	if (!orig)
+		return NULL;
+
+	new = kstrdup_and_replace(orig, '/', ':', GFP_KERNEL);

drivers/gpio/gpiolib-cdev.c +3 -0

+			if (ret) {
+				free_irq_label(label);
 				return ret;
+			}

Quoted 9 lines · Bartosz Golaszewski

From: Kent Gibson <[email protected]>

When adding sanitization of the label, the path through edge_detector_setup() that leads to debounce_setup() was overlooked. A request taking this path does not allocate a new label and the request label is freed twice when the request is released, resulting in memory corruption.

Add label sanitization to debounce_setup().

...

Quoted 8 lines

drivers/gpio/gpiolib-cdev.c +8 -0

+static inline char *make_irq_label(const char *orig)
+{
+     char *new;
+
+     if (!orig)
+             return NULL;
+
+     new = kstrdup_and_replace(orig, '/', ':', GFP_KERNEL);

drivers/gpio/gpiolib-cdev.c static u32 line_event_id(int level) · line 748

Quoted 10 lines

drivers/gpio/gpiolib-cdev.c +10 -0

+     if (!new)
+             return ERR_PTR(-ENOMEM);
+
+     return new;
+}
+
+static inline void free_irq_label(const char *label)
+{
+     kfree(label);
+}

First of all this could have been done in the previous patch already, but okay.

...

drivers/gpio/gpiolib-cdev.c static int debounce_setup(struct line *line, unsigned int debounce_period_us) · line 1066

Quoted 7 lines

drivers/gpio/gpiolib-cdev.c +4 -0

+                     label = make_irq_label(line->req->label);
+                     if (IS_ERR(label))
+                             return -ENOMEM;
+
                      irqflags = IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING;
                      ret = request_irq(irq, debounce_irq_handler, irqflags,
                                        line->req->label, line);

But the main point how does this change fix anything?

Shouldn't be

?

drivers/gpio/gpiolib-cdev.c static int debounce_setup(struct line *line, unsigned int debounce_period_us) · line 1070

Quoted 4 lines

drivers/gpio/gpiolib-cdev.c +3 -0

+                     if (ret) {
+                             free_irq_label(label);
                              return ret;
+                     }

--
With Best Regards,
Andy Shevchenko

From: Bartosz Golaszewski <[email protected]>

This series fixes a couple of bugs in the sanitization of labels being passed to irq.

Patch 1 fixes the case where userspace provides empty labels.

Patch 2 fixes a missed path in the sanitization changes that can result in memory corruption.

v1 -> v2:
- switched the order of the patches in order to avoid introducing buggy code in one just to fix it in the second

Bartosz Golaszewski (1):
gpio: cdev: check for NULL labels when sanitizing them for irqs

Kent Gibson (1):
gpio: cdev: fix missed label sanitizing in debounce_setup()

 drivers/gpio/gpiolib-cdev.c | 46 +++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 15 deletions(-)

--
2.40.1