Input: evdev - add event-mask API
Subsystems (2)
PATCH v3 1/1 Input: evdev - add event-mask API
Hardware manufacturers group keys in the weirdest way possible. This may cause a power-key to be grouped together with normal keyboard keys and thus be reported on the same kernel interface.
However, user-space is often only interested in specific sets of events. For instance, daemons dealing with system-reboot (like systemd-logind) listen for KEY_POWER, but are not interested in any main keyboard keys. Usually, power keys are reported via separate interfaces, however, some i8042 boards report it in the AT matrix. To avoid waking up those system daemons on each key-press, we had two ideas:
- split off KEY_POWER into a separate interface unconditionally
- allow filtering a specific set of events on evdev FDs
Splitting of KEY_POWER is a rather weird way to deal with this and may break backwards-compatibility. It is also specific to KEY_POWER and might be required for other stuff, too. Moreover, we might end up with a huge set of input-devices just to have them properly split.
Hence, this patchset implements the second idea: An event-mask to specify which events you're interested in. Two ioctls allow setting this mask for each event-type. If not set, all events are reported. The type==0 entry is used same as in EVIOCGBIT to set the actual EV_* mask of filtered events. This way, you have a two-level filter.
We are heavily forward-compatible to new event-types and event-codes. So new user-space will be able to run on an old kernel which doesn't know the given event-codes or event-types.
---
Hi Dmitry
This works fine here. Tested with libevdev on my keyboard on x86-64 only. Lemme know if there's anything left to do.
v3:
- fix wording (Dmitry)
- use bits_to_user() (Dmitry)
- add bits_from_user() (David)
- coding style (David)
Thanks a lot!
David
drivers/input/evdev.c | 199 ++++++++++++++++++++++++++++++++++++++++++++- include/uapi/linux/input.h | 59 ++++++++++++++ 2 files changed, 256 insertions(+), 2 deletions(-)
drivers/input/evdev.c +197 -2
@@ -51,10 +51,55 @@ struct evdev_client {struct list_head node;int clkid;bool revoked; +unsigned long *evmasks[EV_CNT ];unsigned int bufsize;struct input_event buffer[]; }; +static size_t evdev_get_mask_cnt (unsigned int type) +{ +static const size_t counts[EV_CNT ] = { +/* EV_SYN==0 is EV_CNT, _not_ SYN_CNT, see EVIOCGBIT */ + [EV_SYN ] =EV_CNT , + [EV_KEY ] =KEY_CNT , + [EV_REL ] =REL_CNT , + [EV_ABS ] =ABS_CNT , + [EV_MSC ] =MSC_CNT , + [EV_SW ] =SW_CNT , + [EV_LED ] =LED_CNT , + [EV_SND ] =SND_CNT , + [EV_FF ] =FF_CNT , + }; + +return (type <EV_CNT ) ? counts[type] :0 ; +} + +/* requires the buffer lock to be held */ +static bool __evdev_is_filtered (struct evdev_client *client, +unsigned int type, +unsigned int code) +{ +unsigned long *mask; +size_t cnt; + +/* EV_SYN and unknown codes are never filtered */ +if (type ==EV_SYN || type >=EV_CNT ) +return false; + +/* first test whether the type is filtered */ + mask = client->evmasks[0 ]; +if (mask && !test_bit (type, mask)) +return true; + +/* unknown values are never filtered */ + cnt =evdev_get_mask_cnt (type); +if (!cnt || code >= cnt) +return false; + + mask = client->evmasks[type]; +return mask && !test_bit (code, mask); +} +/* flush queued events of type @type, caller must hold client->buffer_lock */ static void __evdev_flush_queue (struct evdev_client *client,unsigned int type) {
@@ -176,12 +221,21 @@ static void evdev_pass_values(struct evdev_client *client,spin_lock (&client->buffer_lock);for (v = vals; v != vals + count; v++) { +if (__evdev_is_filtered (client, v->type, v->code)) +continue ; + +if (v->type ==EV_SYN && v->code ==SYN_REPORT ) { +/* drop empty SYN_REPORT */ +if (client->packet_head == client->head) +continue ; + + wakeup = true; + } + event.type = v->type; event.code = v->code; event.value = v->value;__pass_event (client, &event); -if (v->type ==EV_SYN && v->code ==SYN_REPORT ) - wakeup = true; }spin_unlock (&client->buffer_lock);
@@ -364,6 +418,7 @@ static int evdev_release(struct inode *inode, struct file *file) {struct evdev_client *client = file->private_data;struct evdev *evdev = client->evdev; +unsigned int i;mutex_lock (&evdev->mutex);evdev_ungrab (evdev, client);
@@ -371,6 +426,9 @@ static int evdev_release(struct inode *inode, struct file *file)evdev_detach_client (evdev, client); +for (i =0 ; i <EV_CNT ; ++i) +kfree (client->evmasks[i]); +if (is_vmalloc_addr (client))vfree (client);else
@@ -614,6 +672,39 @@ static int bits_to_user(unsigned long *bits, unsigned int maxbit,#endif /* CONFIG_COMPAT */ +static int bits_from_user (unsigned long *bits,unsigned int maxbit, +unsigned int maxlen,const void __user *p,int compat) +{ +int len; + +#if IS_ENABLED (CONFIG_COMPAT ) +if (compat) + len =BITS_TO_LONGS_COMPAT (maxbit) *sizeof (compat_long_t); + else +#endif + len =BITS_TO_LONGS (maxbit) *sizeof (long ); + +if (len > maxlen) + len = maxlen; + +#if IS_ENABLED (CONFIG_COMPAT ) && defined(__BIG_ENDIAN) +if (compat) { +int i; + +for (i =0 ; i < len /sizeof (compat_long_t); i++) +if (copy_from_user ((compat_long_t *) bits + + i +1 - ((i %2 ) <<1 ), + (compat_long_t __user *) p + i, +sizeof (compat_long_t))) +return -EFAULT ; + } else +#endif +if (copy_from_user (bits, p, len)) +return -EFAULT ; + +return len; +} +static int str_to_user (const char *str,unsigned int maxlen,void __user *p) {int len;
@@ -806,6 +897,87 @@ static int evdev_revoke(struct evdev *evdev, struct evdev_client *client,return 0 ; } +/* must be called with evdev-mutex held */ +static int evdev_set_mask (struct evdev_client *client, +unsigned int type, +const void __user *codes, +u32 codes_size, +int compat) +{ +unsigned long flags, *mask, *oldmask; +size_t cnt, size, min; +int error; + +/* we allow unknown types and 'codes_size > size' for forward-compat */ + cnt =evdev_get_mask_cnt (type); +if (!cnt) +return 0 ; + + size =sizeof (unsigned long ) *BITS_TO_LONGS (cnt); + min =min_t (size_t, codes_size, size); + + mask =kzalloc (size,GFP_KERNEL ); +if (!mask) +return -ENOMEM ; + + error =bits_from_user (mask, cnt -1 , min, codes, compat); +if (error <0 ) { +kfree (mask); +return error; + } + +spin_lock_irqsave (&client->buffer_lock, flags); + oldmask = client->evmasks[type]; + client->evmasks[type] = mask; +spin_unlock_irqrestore (&client->buffer_lock, flags); + +kfree (oldmask); + +return 0 ; +} + +/* must be called with evdev-mutex held */ +static int evdev_get_mask (struct evdev_client *client, +unsigned int type, +void __user *codes, +u32 codes_size, +int compat) +{ +unsigned long *mask; +size_t cnt, size, min, i; +u8 __user *out; +int error; + +/* we allow unknown types and 'codes_size > size' for forward-compat */ + cnt =evdev_get_mask_cnt (type); + size =sizeof (unsigned long ) *BITS_TO_LONGS (cnt); + min =min_t (size_t, codes_size, size); + +if (cnt >0 ) { + mask = client->evmasks[type]; +if (mask) { + error =bits_to_user (mask, cnt -1 , min, codes, compat); +if (error <0 ) +return error; + }else { +/* fake mask with all bits set */ + out = (u8 __user *)codes; +for (i =0 ; i < min; ++i) { +if (put_user ((u8 )0xff , out + i)) +return -EFAULT ; + } + } + } + + codes = (u8 *)codes + min; + codes_size -= min; + +if (codes_size >0 &&clear_user (codes, codes_size)) +return -EFAULT ; + +return 0 ; +} +static long evdev_do_ioctl (struct file *file,unsigned int cmd,void __user *p,int compat_mode) {
@@ -813,6 +985,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,struct evdev *evdev = client->evdev;struct input_dev *dev = evdev->handle.dev;struct input_absinfo abs; +struct input_mask mask;struct ff_effect effect;int __user *ip = (int __user *)p;unsigned int i, t, u, v;
@@ -874,6 +1047,28 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,else return evdev_revoke (evdev, client, file); +case EVIOCGMASK : +if (copy_from_user (&mask, p,sizeof (mask))) +return -EFAULT ; + +return evdev_get_mask (client, + mask.type, + (void __user *) + (unsigned long )mask.codes_ptr, + mask.codes_size, + compat_mode); + +case EVIOCSMASK : +if (copy_from_user (&mask, p,sizeof (mask))) +return -EFAULT ; + +return evdev_set_mask (client, + mask.type, + (const void __user *) + (unsigned long )mask.codes_ptr, + mask.codes_size, + compat_mode); +case EVIOCSCLOCKID :if (copy_from_user (&i, p,sizeof (unsigned int )))return -EFAULT ;
include/uapi/linux/input.h +59 -0
71 lines not on this page · raw message
Signature · 1 line
2.1.3
Dmitry Torokhov
maintainerHi David,
drivers/input/evdev.c static int bits_to_user(unsigned long *bits, unsigned int maxbit, · line 681
Quoted 7 lines · David Herrmann
drivers/input/evdev.c +7
-0+static int bits_from_user (unsigned long *bits,unsigned int maxbit, +unsigned int maxlen,const void __user *p,int compat) +{ +int len; + +#if IS_ENABLED (CONFIG_COMPAT ) +if (compat)
I think this can be written as:
if (IS_ENABLED(CONFIG_COMPAT) && compat)
Quoted 7 lines
drivers/input/evdev.c +7
-0+ len =BITS_TO_LONGS_COMPAT (maxbit) *sizeof (compat_long_t); + else +#endif + len =BITS_TO_LONGS (maxbit) *sizeof (long ); + +if (len > maxlen) + len = maxlen;
drivers/input/evdev.c static int bits_to_user(unsigned long *bits, unsigned int maxbit, · line 698
Quoted 10 lines
drivers/input/evdev.c +10
-0+ +#if IS_ENABLED (CONFIG_COMPAT ) && defined(__BIG_ENDIAN) +if (compat) { +int i; + +for (i =0 ; i < len /sizeof (compat_long_t); i++) +if (copy_from_user ((compat_long_t *) bits + + i +1 - ((i %2 ) <<1 ), + (compat_long_t __user *) p + i, +sizeof (compat_long_t)))
Unfortunately this is not quite enough, you need to also zero out the upper bits of the last partial if userspace did not supply multiple of 64 bits.
Frankly, bitmask APIs that we have in input are god-awful on big endian:
they do not work if usespace uses quantities less than long (or compat long). I think we'll have to enforce it in the code.
drivers/input/evdev.c static int evdev_revoke(struct evdev *evdev, struct evdev_client *client, · line 967
Quoted 6 lines
drivers/input/evdev.c +6
-0+ }else { +/* fake mask with all bits set */ + out = (u8 __user *)codes; +for (i =0 ; i < min; ++i) { +if (put_user ((u8 )0xff , out + i)) +return -EFAULT ;
I wonder if we should not replace this with access_ok() + straight memset instead of doing byte-by-byte copy.
Thanks.
Signature · 1 line
Dmitry
Hi
Quoted 12 lines · Dmitry Torokhov
Hi David,
drivers/input/evdev.c static int bits_to_user(unsigned long *bits, unsigned int maxbit, · line 681
Quoted 7 lines · David Herrmann
drivers/input/evdev.c +7
-0+static int bits_from_user (unsigned long *bits,unsigned int maxbit, +unsigned int maxlen,const void __user *p,int compat) +{ +int len; + +#if IS_ENABLED (CONFIG_COMPAT ) +if (compat)I think this can be written as:
Quoted 1 line
if (IS_ENABLED(CONFIG_COMPAT) && compat)
BITS_TO_LONGS_COMPAT is only defined if CONFIG_COMPAT is. We'd rely on compiler-optimizations (dead code elimination) if we'd do that change. I'm not really fond of making -O0 compiles fail, but there're several places in the kernel that do. Your call ;)
Quoted 22 lines
Quoted 7 lines
drivers/input/evdev.c +7
-0+ len =BITS_TO_LONGS_COMPAT (maxbit) *sizeof (compat_long_t); + else +#endif + len =BITS_TO_LONGS (maxbit) *sizeof (long ); + +if (len > maxlen) + len = maxlen;drivers/input/evdev.c static int bits_to_user(unsigned long *bits, unsigned int maxbit, · line 698
Quoted 10 lines
drivers/input/evdev.c +10
-0+ +#if IS_ENABLED (CONFIG_COMPAT ) && defined(__BIG_ENDIAN) +if (compat) { +int i; + +for (i =0 ; i < len /sizeof (compat_long_t); i++) +if (copy_from_user ((compat_long_t *) bits + + i +1 - ((i %2 ) <<1 ), + (compat_long_t __user *) p + i, +sizeof (compat_long_t)))Unfortunately this is not quite enough, you need to also zero out the upper bits of the last partial if userspace did not supply multiple of 64 bits.
Quoted 3 lines
Frankly, bitmask APIs that we have in input are god-awful on big endian:
they do not work if usespace uses quantities less than long (or compat long). I think we'll have to enforce it in the code.
Right, I relied on the caller of bits_from_user() to initialize it to zero and make it long-aligned. I can move it into that helper.
Quoted 9 lines
drivers/input/evdev.c static int evdev_revoke(struct evdev *evdev, struct evdev_client *client, · line 967
Quoted 6 lines
drivers/input/evdev.c +6
-0+ }else { +/* fake mask with all bits set */ + out = (u8 __user *)codes; +for (i =0 ; i < min; ++i) { +if (put_user ((u8 )0xff , out + i)) +return -EFAULT ;I wonder if we should not replace this with
access_ok()+ straight memset instead of doing byte-by-byte copy.
I'm actually not very familiar with the details of copy_to_user(). That's why I went with put_user(). I can look into it, but if you know it well, I am open for suggestions.
Thanks
David
Hi
Quoted 45 lines · David Herrmann
Hi
Quoted 12 lines · Dmitry Torokhov
Hi David,
drivers/input/evdev.c static int bits_to_user(unsigned long *bits, unsigned int maxbit, · line 681
Quoted 7 lines · David Herrmann
drivers/input/evdev.c +7
-0+static int bits_from_user (unsigned long *bits,unsigned int maxbit, +unsigned int maxlen,const void __user *p,int compat) +{ +int len; + +#if IS_ENABLED (CONFIG_COMPAT ) +if (compat)I think this can be written as:
Quoted 1 line
if (IS_ENABLED(CONFIG_COMPAT) && compat)
BITS_TO_LONGS_COMPATis only defined ifCONFIG_COMPATis. We'd rely on compiler-optimizations (dead code elimination) if we'd do that change. I'm not really fond of making -O0 compiles fail, but there're several places in the kernel that do. Your call ;)Quoted 22 lines
Quoted 7 lines
drivers/input/evdev.c +7
-0+ len =BITS_TO_LONGS_COMPAT (maxbit) *sizeof (compat_long_t); + else +#endif + len =BITS_TO_LONGS (maxbit) *sizeof (long ); + +if (len > maxlen) + len = maxlen;drivers/input/evdev.c static int bits_to_user(unsigned long *bits, unsigned int maxbit, · line 698
Quoted 10 lines
drivers/input/evdev.c +10
-0+ +#if IS_ENABLED (CONFIG_COMPAT ) && defined(__BIG_ENDIAN) +if (compat) { +int i; + +for (i =0 ; i < len /sizeof (compat_long_t); i++) +if (copy_from_user ((compat_long_t *) bits + + i +1 - ((i %2 ) <<1 ), + (compat_long_t __user *) p + i, +sizeof (compat_long_t)))Unfortunately this is not quite enough, you need to also zero out the upper bits of the last partial if userspace did not supply multiple of 64 bits.
Quoted 6 lines
Quoted 3 lines
Frankly, bitmask APIs that we have in input are god-awful on big endian:
they do not work if usespace uses quantities less than long (or compat long). I think we'll have to enforce it in the code.Right, I relied on the caller of
bits_from_user()to initialize it to zero and make it long-aligned. I can move it into that helper.
I now added a memset(bits, 0, len); before "if (len > maxlen)". This just clears the buffer before copying data. Due to the cross-over in the for()-loop, I'm not entirely sure how to only clear the unused parts, as the tail "long"s might be only set partially. Quite confusing..
I didn't change anything else. Lemme know if that's fine and I will resend the patch.
Thanks
David
Quoted 16 lines
Quoted 9 lines
drivers/input/evdev.c static int evdev_revoke(struct evdev *evdev, struct evdev_client *client, · line 967
Quoted 6 lines
drivers/input/evdev.c +6
-0+ }else { +/* fake mask with all bits set */ + out = (u8 __user *)codes; +for (i =0 ; i < min; ++i) { +if (put_user ((u8 )0xff , out + i)) +return -EFAULT ;I wonder if we should not replace this with
access_ok()+ straight memset instead of doing byte-by-byte copy.I'm actually not very familiar with the details of
copy_to_user(). That's why I went withput_user(). I can look into it, but if you know it well, I am open for suggestions.Thanks
David