| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120113091556.GA21749@core.coreip.homeip.net>
Date: Fri, 13 Jan 2012 01:15:56 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Chase Douglas <chase.douglas@...onical.com>
Cc: Henrik Rydberg <rydberg@...omail.se>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] Input: Add EVIOC mechanism for MT slots
On Fri, Jan 13, 2012 at 09:34:12AM +0100, Chase Douglas wrote:
> On 01/12/2012 08:24 PM, Henrik Rydberg wrote:
> > This patch adds the ability to extract MT slot data via a new ioctl,
> > EVIOCGMTSLOTS. The function returns an array of slot values for the
> > specified ABS_MT event type.
> >
> > Example of user space usage:
> >
> > struct { struct input_mt_request req; int values[64]; } data;
> > data.req.code = ABS_MT_POSITION_X;
> > if (ioctl(fd, EVIOCGMTSLOTS(sizeof(data)), &data) < 0)
> > return -1;
> > for (i = 0; i < data.req.num_values; i++)
> > printf("slot %d: %d\n", i, data.values[i]);
>
> If input_mt_request is:
>
> struct input_mt_request {
> __u32 code;
> __u32 num_values;
> __s32 values[];
> };
>
> then how passing in the data struct above work? The passed in data would be:
>
> __u32 code;
> __u32 num_values; /* BTW, this isn't set in the example above */
> __s32 *values; /* This is left uninitialized in the example above */
> int values[0]
> int values[1]
> int values[2]
> ...
> int values[63]
>
> I'm not too familiar with ioctl interfaces, but I don't see how this
> example could work.
There isn't a separate __s32 *values pointer, the actual memory layout
from kernel's POV would be:
__u32 code;
__u32 num_values;
__s32 values[0];
__s32 values[1];
...
__s32 values[63];
>
> >
> > Cc: Chase Douglas <chase.douglas@...onical.com>
> > Signed-off-by: Henrik Rydberg <rydberg@...omail.se>
> > ---
> > Hi Dmitry,
> >
> > This is a new take on the MT slot ioctl patch. A new ioctl is added,
> > which takes a request structure and returns a (variable) array of slot
> > values. The patch seems to work, but testing has not been extensive (yet).
> >
> > Cheers,
> > Henrik
> >
> > drivers/input/evdev.c | 30 +++++++++++++++++++++++++++++-
> > include/linux/input.h | 25 +++++++++++++++++++++++++
> > 2 files changed, 54 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > index 4cf2534..8bf45d4 100644
> > --- a/drivers/input/evdev.c
> > +++ b/drivers/input/evdev.c
> > @@ -20,7 +20,7 @@
> > #include <linux/slab.h>
> > #include <linux/module.h>
> > #include <linux/init.h>
> > -#include <linux/input.h>
> > +#include <linux/input/mt.h>
> > #include <linux/major.h>
> > #include <linux/device.h>
> > #include "input-compat.h"
> > @@ -621,6 +621,31 @@ static int evdev_handle_set_keycode_v2(struct input_dev *dev, void __user *p)
> > return input_set_keycode(dev, &ke);
> > }
> >
> > +static int evdev_handle_mt_request(struct input_dev *dev,
> > + unsigned int size,
> > + int __user *ip)
> > +{
> > + const struct input_mt_slot *mt = dev->mt;
> > + unsigned int code;
> > + int i;
> > +
> > + if (get_user(code, &ip[0]))
> > + return -EFAULT;
> > + if (!input_is_mt_value(code))
> > + return -EINVAL;
> > +
> > + if (size < sizeof(struct input_mt_request) + dev->mtsize * sizeof(int))
> > + return -EINVAL;
>
> Why not fill in up to the size of the array provided?
Yes, I think we do the same with other ioctls as well.
>
> > + if (put_user(dev->mtsize, &ip[1]))
> > + return -EFAULT;
> > + for (i = 0; i < dev->mtsize; i++)
> > + if (put_user(input_mt_get_value(&mt[i], code), &ip[2 + i]))
> > + return -EFAULT;
>
> This would be easier to understand if you did:
>
> struct input_mt_request *req = (struct input_mt_request *)ip;
>
> Then modify the stuff in a straightforward way.
This is a user pointer, you can't really do it in straightforward way...
>
> > + return 0;
> > +}
> > +
> > static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > void __user *p, int compat_mode)
> > {
> > @@ -706,6 +731,9 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
> > return bits_to_user(dev->propbit, INPUT_PROP_MAX,
> > size, p, compat_mode);
> >
> > + case EVIOCGMTSLOTS(0):
> > + return evdev_handle_mt_request(dev, size, ip);
> > +
> > case EVIOCGKEY(0):
> > return bits_to_user(dev->key, KEY_MAX, size, p, compat_mode);
> >
> > diff --git a/include/linux/input.h b/include/linux/input.h
> > index a637e78..9a73f18 100644
> > --- a/include/linux/input.h
> > +++ b/include/linux/input.h
> > @@ -99,6 +99,30 @@ struct input_keymap_entry {
> > __u8 scancode[32];
> > };
> >
> > +/**
> > + * struct input_mt_request - used by EVIOCGMTSLOTS ioctl
> > + * @code: ABS_MT code to read
> > + * @num_values: number of values written to the value array
> > + * @values: value array, one value per slot
> > + *
> > + * The structure is used to retrieve MT slot data. Before the call,
> > + * code is set to the wanted ABS_MT event type. On return, the value
> > + * array is filled with the slot values for the specified ABS_MT code,
> > + * and num_values is set to the actual number of slots.
> > + *
> > + * The call argument (len) is the size of the return buffer, satisfying
> > + *
> > + * len >= sizeof(input_mt_request) + sizeof(__s32) * number_of_slots
> > + *
> > + * If the request code is not an ABS_MT value, or if len is too small,
> > + * -EINVAL is returned.
> > + */
> > +struct input_mt_request {
> > + __u32 code;
> > + __u32 num_values;
Hmm, might avoid this one... Userspace can do EVIOCGABS to get number of
slots and then it will know how much data EVIOCGMTSLOTS will return...
> > + __s32 values[];
> > +};
> > +
> > #define EVIOCGVERSION _IOR('E', 0x01, int) /* get driver version */
> > #define EVIOCGID _IOR('E', 0x02, struct input_id) /* get device ID */
> > #define EVIOCGREP _IOR('E', 0x03, unsigned int[2]) /* get repeat settings */
> > @@ -113,6 +137,7 @@ struct input_keymap_entry {
> > #define EVIOCGPHYS(len) _IOC(_IOC_READ, 'E', 0x07, len) /* get physical location */
> > #define EVIOCGUNIQ(len) _IOC(_IOC_READ, 'E', 0x08, len) /* get unique identifier */
> > #define EVIOCGPROP(len) _IOC(_IOC_READ, 'E', 0x09, len) /* get device properties */
> > +#define EVIOCGMTSLOTS(len) _IOC(_IOC_READ, 'E', 0x0a, len) /* get MT slot values */
> >
> > #define EVIOCGKEY(len) _IOC(_IOC_READ, 'E', 0x18, len) /* get global key state */
> > #define EVIOCGLED(len) _IOC(_IOC_READ, 'E', 0x19, len) /* get all LEDs */
>
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists