gpiolib: linehandle_ioctl() tweaks
cover gpiolib: linehandle_ioctl() tweaks
I noticed that - unlike sysfs - it's possible to set values of input lines from user space using the character device. There's actually even a TODO in the linehandle_ioctl() code.
This is not correct so the first patches fixes that and the second removes a TODO that's not really needed.
Bartosz Golaszewski (2):
gpiolib: don't allow userspace to set values of input lines
gpiolib: remove an unneccessary TODO
drivers/gpio/gpiolib.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Signature · 1 line
2.17.1
PATCH 1/2 gpiolib: don't allow userspace to set values of input lines
User space can currently both read and set values of input lines using the character device. This was not allowed by the old sysfs interface nor is it a correct behavior.
Check the first descriptor in the set for the OUT flag when asked to set values and return -EPERM if the line is input.
--- drivers/gpio/gpiolib.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/gpio/gpiolib.c +7 -1
@@ -449,7 +449,13 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,return 0 ; }else if (cmd ==GPIOHANDLE_SET_LINE_VALUES_IOCTL ) { -/* TODO: check if descriptors are really output */ +/* +* All line descriptors were created at once with the same +* flags so just check if the first one is really output. +*/ +if (!test_bit (FLAG_IS_OUT , &lh->descs[0 ]->flags)) +return -EPERM ; +if (copy_from_user (&ghd, ip,sizeof (ghd)))return -EFAULT ;
Signature · 1 line
2.17.1
Re: [PATCH 1/2] gpiolib: don't allow userspace to set values of input lines
Quoted 8 lines · Bartosz Golaszewski
User space can currently both read and set values of input lines using the character device. This was not allowed by the old sysfs interface nor is it a correct behavior.
Check the first descriptor in the set for the OUT flag when asked to set values and return
-EPERMif the line is input.Signed-off-by Bartosz Golaszewski <[email protected]>
Patch applied! Thanks for fixing this.
Quoted 2 lines
} else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) {- /* TODO: check if descriptors are really output */
I wonder what kind of lazy coder leaves this kind of garbage behind for others to fix up...
d7c51b47ac11e (Linus Walleij 2016-04-26 10:35:29 +0200 451)
} else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) {d7c51b47ac11e (Linus Walleij 2016-04-26 10:35:29 +0200 452)
/* TODO: check if descriptors are really output */
d7c51b47ac11e (Linus Walleij 2016-04-26 10:35:29 +0200 453)
if (copy_from_user(&ghd, ip, sizeof(ghd)))Oh that guy.
Yours,
Linus Walleij
Re: [PATCH 1/2] gpiolib: don't allow userspace to set values of input lines
2018-07-16 15:40 GMT+02:00 Linus Walleij <[email protected]>:
Quoted 6 lines · Bartosz Golaszewski
Quoted 5 lines
User space can currently both read and set values of input lines using the character device. This was not allowed by the old sysfs interface nor is it a correct behavior.
Quoted 7 lines
Quoted 4 lines
Check the first descriptor in the set for the OUT flag when asked to set values and return
-EPERMif the line is input.Signed-off-by Bartosz Golaszewski <[email protected]>Patch applied! Thanks for fixing this.
Thanks. Do you think we should Cc stable on that one?
Bart
Re: [PATCH 1/2] gpiolib: don't allow userspace to set values of input lines
Quoted 7 lines · Bartosz Golaszewski
2018-07-16 15:40 GMT+02:00 Linus Walleij <[email protected]>:
Quoted 6 lines · Bartosz Golaszewski
Quoted 5 lines
User space can currently both read and set values of input lines using the character device. This was not allowed by the old sysfs interface nor is it a correct behavior.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Check the first descriptor in the set for the OUT flag when asked to set values and return
-EPERMif the line is input.Signed-off-by Bartosz Golaszewski <[email protected]>Patch applied! Thanks for fixing this.
Thanks. Do you think we should Cc stable on that one?
Nah. It's not a regression. (Not like things that worked before stopped working.)
Yours,
Linus Walleij
PATCH 2/2 gpiolib: remove an unnecessary TODO
In reply to Bartosz GolaszewskiIt's actually fine to read values of output lines. This was also allowed by the legacy sysfs interface.
--- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpiolib.c +1 -1
@@ -431,7 +431,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,int i;if (cmd ==GPIOHANDLE_GET_LINE_VALUES_IOCTL ) { -/* TODO: check if descriptors are really input */ +/* NOTE: It's ok to read values of output lines. */ int ret = gpiod_get_array_value_complex(false, true, lh->numdescs,
Signature · 1 line
2.17.1
Re: [PATCH 2/2] gpiolib: remove an unnecessary TODO
Quoted 4 lines · Bartosz Golaszewski
It's actually fine to read values of output lines. This was also allowed by the legacy sysfs interface.
Signed-off-by Bartosz Golaszewski <[email protected]>
Patch applied.
Yours,
Linus Walleij