gpiolib: improvements in linehandle_create()

PATCH In review Bartosz Golaszewski 2 participants 4 messages Oct 16, 2017 GPIO

Share & tools

Bartosz Golaszewski

author

2017-10-16 09:32 UTC #0 lore raw

cover gpiolib: improvements in linehandle_create()

Two minor fixes for creating user space line handles. The first patch simplifies checking the request flags. The second adds an additional sanitizing check.

Bartosz Golaszewski (2):
gpiolib: only check line handle flags once
gpiolib: don't allow OPEN_DRAIN & OPEN_SOURCE flags for input

 drivers/gpio/gpiolib.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Signature · 1 line

2.13.2

Bartosz Golaszewski

author

2017-10-16 09:32 UTC #1 lore raw

PATCH 1/2 gpiolib: only check line handle flags once

There's no need to check the validity of handle request flags more than once, right after copying the data from user. Move the check out of the for loop and simplify the error path by bailing out before allocating any resources.

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

---
 drivers/gpio/gpiolib.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/gpio/gpiolib.c +7 -7

index eb80dac4e26a..5d98345c9dc8 100644
@@ -444,12 +444,19 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	struct linehandle_state *lh;
 	struct file *file;
 	int fd, i, ret;
+	u32 lflags;
 
 	if (copy_from_user(&handlereq, ip, sizeof(handlereq)))
 		return -EFAULT;
 	if ((handlereq.lines == 0) || (handlereq.lines > GPIOHANDLES_MAX))
 		return -EINVAL;
 
+	lflags = handlereq.flags;
+
+	/* Return an error if an unknown flag is set */
+	if (lflags & ~GPIOHANDLE_REQUEST_VALID_FLAGS)
+		return -EINVAL;
+
 	lh = kzalloc(sizeof(*lh), GFP_KERNEL);
 	if (!lh)
 		return -ENOMEM;

@@ -470,7 +477,6 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	/* Request each GPIO */
 	for (i = 0; i < handlereq.lines; i++) {
 		u32 offset = handlereq.lineoffsets[i];
-		u32 lflags = handlereq.flags;
 		struct gpio_desc *desc;
 
 		if (offset >= gdev->ngpio) {

@@ -478,12 +484,6 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 			goto out_free_descs;
 		}
 
-		/* Return an error if a unknown flag is set */
-		if (lflags & ~GPIOHANDLE_REQUEST_VALID_FLAGS) {
-			ret = -EINVAL;
-			goto out_free_descs;
-		}
-
 		desc = &gdev->descs[offset];
 		ret = gpiod_request(desc, lh->label);
 		if (ret)

Signature · 1 line

2.13.2

Bartosz Golaszewski

author

2017-10-16 09:32 UTC #2 lore raw

PATCH 2/2 gpiolib: don't allow OPEN_DRAIN & OPEN_SOURCE flags for input

In reply to Bartosz Golaszewski

OPEN_DRAIN and OPEN_SOURCE flags only affect the way we drive a GPIO line, so they only make sense for output mode. Just as we only allow input mode for event handle requests, don't allow passing open-drain and open-source flags for any other mode than explicit output.

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

---
 drivers/gpio/gpiolib.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/gpio/gpiolib.c +6 -0

index 5d98345c9dc8..eac86ead6cfa 100644
@@ -457,6 +457,12 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	if (lflags & ~GPIOHANDLE_REQUEST_VALID_FLAGS)
 		return -EINVAL;
 
+	/* OPEN_DRAIN and OPEN_SOURCE flags only make sense for output mode. */
+	if (!(lflags & GPIOHANDLE_REQUEST_OUTPUT) &&
+	    ((lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN) ||
+	     (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)))
+		return -EINVAL;
+
 	lh = kzalloc(sizeof(*lh), GFP_KERNEL);
 	if (!lh)
 		return -ENOMEM;

Signature · 1 line

2.13.2

Linus Walleij

2017-10-16 11:47 UTC #3 lore raw

In reply to Bartosz Golaszewski

Quoted 3 lines · Bartosz Golaszewski

Two minor fixes for creating user space line handles. The first patch simplifies checking the request flags. The second adds an additional sanitizing check.

Patches makes all kind of sense, so applied.

Yours,
Linus Walleij