If an input device declares too many capability bits then modalias
string for such device may become too long and not fit into uevent
buffer, resulting in failure of sending said uevent. This, in turn,
may prevent userspace from recognizing existence of such devices.
This is typically not a concern for real hardware devices as they have
limited number of keys, but happen with synthetic devices such as
ones created by xen-kbdfront driver, which creates devices as being
capable of delivering all possible keys, since it doesn't know what
keys the backend may produce.
To deal with such devices input core will attempt to trim key data,
in the hope that the rest of modalias string will fit in the given
buffer. When trimming key data it will indicate that it is not
complete by placing "+," sign, resulting in conversions like this:
old: k71,72,73,74,78,7A,7B,7C,7D,8E,9E,A4,AD,E0,E1,E4,F8,174,
new: k71,72,73,74,78,7A,7B,7C,+,
This should allow existing udev rules continue to work with existing
devices, and will also allow writing more complex rules that would
recognize trimmed modalias and check input device characteristics by
other means (for example by parsing KEY= data in uevent or parsing
input device sysfs attributes).
Reported-by: Jason Andryuk <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/input.c | 85 +++++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 15 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index b04bcdeee557..947b514174e4 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1338,19 +1338,19 @@ static int input_print_modalias_bits(char *buf, int size,
char name, const unsigned long *bm,
unsigned int min_bit, unsigned int max_bit)
{
- int len = 0, i;
+ int bit = min_bit;
+ int len = 0;
len += snprintf(buf, max(size, 0), "%c", name);
- for (i = min_bit; i < max_bit; i++)
- if (bm[BIT_WORD(i)] & BIT_MASK(i))
- len += snprintf(buf + len, max(size - len, 0), "%X,", i);
+ for_each_set_bit_from(bit, bm, max_bit)
+ len += snprintf(buf + len, max(size - len, 0), "%X,", bit);
return len;
}
-static int input_print_modalias(char *buf, int size, const struct input_dev *id,
- int add_cr)
+static int input_print_modalias_parts(char *buf, int size, int full_len,
+ const struct input_dev *id)
{
- int len;
+ int len, klen, remainder, space;
len = snprintf(buf, max(size, 0),
"input:b%04Xv%04Xp%04Xe%04X-",
@@ -1359,8 +1359,48 @@ static int input_print_modalias(char *buf, int size, const struct input_dev *id,
len += input_print_modalias_bits(buf + len, size - len,
'e', id->evbit, 0, EV_MAX);
- len += input_print_modalias_bits(buf + len, size - len,
+
+ /*
+ * Calculate the remaining space in the buffer making sure we
+ * have place for the terminating 0.
+ */
+ space = max(size - (len + 1), 0);
+
+ klen = input_print_modalias_bits(buf + len, size - len,
'k', id->keybit, KEY_MIN_INTERESTING, KEY_MAX);
+ len += klen;
+
+ /*
+ * If we have more data than we can fit in the buffer, check
+ * if we can trim key data to fit in the rest. We will indicate
+ * that key data is incomplete by adding "+" sign at the end, like
+ * this: * "k1,2,3,45,+,".
+ *
+ * Note that we shortest key info (if present) is "k+," so we
+ * can only try to trim if key data is longer than that.
+ */
+ if (full_len && size < full_len + 1 && klen > 3) {
+ remainder = full_len - len;
+ /*
+ * We can only trim if we have space for the remainder
+ * and also for at least "k+," which is 3 more characters.
+ */
+ if (remainder <= space - 3) {
+ /*
+ * We are guaranteed to have 'k' in the buffer, so
+ * we need at least 3 additional bytes for storing
+ * "+," in addition to the remainder.
+ */
+ for (int i = size - 1 - remainder - 3; i >= 0; i--) {
+ if (buf[i] == 'k' || buf[i] == ',') {
+ strcpy(buf + i + 1, "+,");
+ len = i + 3; /* Not counting '\0' */
+ break;
+ }
+ }
+ }
+ }
+
len += input_print_modalias_bits(buf + len, size - len,
'r', id->relbit, 0, REL_MAX);
len += input_print_modalias_bits(buf + len, size - len,
@@ -1376,22 +1416,37 @@ static int input_print_modalias(char *buf, int size, const struct input_dev *id,
len += input_print_modalias_bits(buf + len, size - len,
'w', id->swbit, 0, SW_MAX);
- if (add_cr)
- len += snprintf(buf + len, max(size - len, 0), "\n");
-
return len;
}
+static int input_print_modalias(char *buf, int size, const struct input_dev *id)
+{
+ int full_len;
+
+ /*
+ * Printing is done in 2 passes: first one figures out total length
+ * needed for the modalias string, second one will try to trim key
+ * data in case when buffer is too small for the entire modalias.
+ * If the buffer is too small regardless, it will fill as much as it
+ * can (without trimming key data) into the buffer and leave it to
+ * the caller to figure out what to do with the result.
+ */
+ full_len = input_print_modalias_parts(NULL, 0, 0, id);
+ return input_print_modalias_parts(buf, size, full_len, id);
+}
+
static ssize_t input_dev_show_modalias(struct device *dev,
struct device_attribute *attr,
char *buf)
{
struct input_dev *id = to_input_dev(dev);
- ssize_t len;
+ size_t len;
- len = input_print_modalias(buf, PAGE_SIZE, id, 1);
+ len = input_print_modalias(buf, PAGE_SIZE, id);
+ if (len < PAGE_SIZE - 2)
+ len += snprintf(buf + len, PAGE_SIZE - len, "\n");
- return min_t(int, len, PAGE_SIZE);
+ return min(len, PAGE_SIZE);
}
static DEVICE_ATTR(modalias, S_IRUGO, input_dev_show_modalias, NULL);
@@ -1611,7 +1666,7 @@ static int input_add_uevent_modalias_var(struct kobj_uevent_env *env,
len = input_print_modalias(&env->buf[env->buflen - 1],
sizeof(env->buf) - env->buflen,
- dev, 0);
+ dev);
if (len >= (sizeof(env->buf) - env->buflen))
return -ENOMEM;
--
2.44.0.769.g3c40516874-goog
--
Dmitry
Hi Dmitry,
On Sat, Apr 27, 2024 at 10:06 PM Dmitry Torokhov
<[email protected]> wrote:
>
> If an input device declares too many capability bits then modalias
> string for such device may become too long and not fit into uevent
> buffer, resulting in failure of sending said uevent. This, in turn,
> may prevent userspace from recognizing existence of such devices.
>
> This is typically not a concern for real hardware devices as they have
> limited number of keys, but happen with synthetic devices such as
> ones created by xen-kbdfront driver, which creates devices as being
> capable of delivering all possible keys, since it doesn't know what
> keys the backend may produce.
>
> To deal with such devices input core will attempt to trim key data,
> in the hope that the rest of modalias string will fit in the given
> buffer. When trimming key data it will indicate that it is not
> complete by placing "+," sign, resulting in conversions like this:
>
> old: k71,72,73,74,78,7A,7B,7C,7D,8E,9E,A4,AD,E0,E1,E4,F8,174,
> new: k71,72,73,74,78,7A,7B,7C,+,
>
> This should allow existing udev rules continue to work with existing
> devices, and will also allow writing more complex rules that would
> recognize trimmed modalias and check input device characteristics by
> other means (for example by parsing KEY= data in uevent or parsing
> input device sysfs attributes).
I think adding these links may be useful for cross referencing:
[1] https://github.com/systemd/systemd/issues/22944
[2] https://lore.kernel.org/xen-devel/[email protected]/T/
> Reported-by: Jason Andryuk <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Thank you for looking at this. I think this idea of truncating the
modalias is better than just dropping keys.
cat-ing the individual sysfs files works, but there is still an issue:
# sudo udevadm trigger --action=change
[ 601.379977] ------------[ cut here ]------------
[ 601.395959] add_uevent_var: buffer size too small
[ 601.412009] WARNING: CPU: 0 PID: 630 at lib/kobject_uevent.c:671
add_uevent_var+0x11c/0x130
[ 601.440379] Modules linked in: xen_kbdfront xen_blkfront xen_netfront
[ 601.462078] CPU: 0 PID: 630 Comm: udevadm Tainted: G W
6.8.7+ #2
[ 601.486003] Hardware name: Xen HVM domU, BIOS 4.19-unstable 03/09/2024
[ 601.504867] RIP: 0010:add_uevent_var+0x11c/0x130
[ 601.527988] Code: 5b 41 5c 5d c3 cc cc cc cc 48 c7 c7 c0 3c 4d 9e
e8 49 4c 1c ff 0f 0b b8 f4 ff ff ff eb ce 48 c7 c7 e8 3c 4d 9e e8 34
4c 1c ff <0f> 0b eb e9 e8 eb e0 02 00 66 66 2e 0f 1f 84 00 00 00 00 00
90 90
[ 601.590038] RSP: 0018:ffffadc60053bcf0 EFLAGS: 00010282
[ 601.612133] RAX: 0000000000000000 RBX: ffff96f943c0a000 RCX: 0000000000000000
[ 601.634794] RDX: ffff96f9bd428cd0 RSI: ffff96f9bd41d740 RDI: ffff96f9bd41d740
[ 601.651867] RBP: ffffadc60053bd50 R08: 00000000ffffdfff R09: 0000000000000001
[ 601.677718] R10: 00000000ffffdfff R11: ffffffff9e65cc00 R12: 0000000000000003
[ 601.699194] R13: ffff96f943c0a000 R14: ffffffff9e0db1d0 R15: 0000000000000000
[ 601.718038] FS: 00007fa9a1084d40(0000) GS:ffff96f9bd400000(0000)
knlGS:0000000000000000
[ 601.741494] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 601.761050] CR2: 00007fff40d8bd58 CR3: 0000000002712001 CR4: 00000000000606f0
[ 601.783095] Call Trace:
[ 601.791569] <TASK>
[ 601.798207] ? add_uevent_var+0x11c/0x130
[ 601.810481] ? __warn+0x7c/0x130
[ 601.822437] ? add_uevent_var+0x11c/0x130
[ 601.833016] ? report_bug+0x171/0x1a0
[ 601.848035] ? handle_bug+0x3c/0x80
[ 601.858013] ? exc_invalid_op+0x17/0x70
[ 601.873026] ? asm_exc_invalid_op+0x1a/0x20
[ 601.888058] ? add_uevent_var+0x11c/0x130
[ 601.899042] kobject_uevent_env+0x28e/0x510
[ 601.916043] kobject_synth_uevent+0x326/0x330
[ 601.927373] uevent_store+0x19/0x50
[ 601.940381] kernfs_fop_write_iter+0x122/0x1b0
[ 601.952343] vfs_write+0x299/0x470
[ 601.961853] ksys_write+0x6a/0xf0
[ 601.975611] do_syscall_64+0x52/0x120
[ 601.985363] entry_SYSCALL_64_after_hwframe+0x78/0x80
[ 602.001942] RIP: 0033:0x7fa9a18693b4
[ 602.058258] Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b3 0f
1f 80 00 00 00 00 48 8d 05 49 53 0d 00 8b 00 85 c0 75 13 b8 01 00 00
00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 41 54 49 89 d4 55 48 89
f5 53
[ 602.107044] RSP: 002b:00007ffc204b01e8 EFLAGS: 00000246 ORIG_RAX:
0000000000000001
[ 602.125072] RAX: ffffffffffffffda RBX: 0000000000000007 RCX: 00007fa9a18693b4
[ 602.144401] RDX: 0000000000000007 RSI: 00007ffc204b02d0 RDI: 0000000000000003
[ 602.161037] RBP: 00007ffc204b02d0 R08: 00005641874fbcd0 R09: 0000564187578700
[ 602.183535] R10: 0000000000000000 R11: 0000000000000246 R12: 00005641874fbbf0
[ 602.201557] R13: 0000000000000007 R14: 00007fa9a1935760 R15: 0000000000000007
[ 602.220506] </TASK>
[ 602.227408] ---[ end trace 0000000000000000 ]---
[ 602.239544] synth uevent: /devices/virtual/input/input5: failed to
send uevent
[ 602.260848] input input5: uevent: failed to send synthetic uevent: -12
Another path needs to truncate the buffer? Or the problem is that the
total uevent buffer size is what matters - not just the keys modalias?
My other thought is wondering whether the presence of '+' will cause
parsing errors? Has '+' been used before - or will it be an
unexpected character?
Thanks,
Jason
On Sun, Apr 28, 2024 at 07:43:52PM -0400, Jason Andryuk wrote:
> Hi Dmitry,
>
> On Sat, Apr 27, 2024 at 10:06 PM Dmitry Torokhov
> <[email protected]> wrote:
> >
> > If an input device declares too many capability bits then modalias
> > string for such device may become too long and not fit into uevent
> > buffer, resulting in failure of sending said uevent. This, in turn,
> > may prevent userspace from recognizing existence of such devices.
> >
> > This is typically not a concern for real hardware devices as they have
> > limited number of keys, but happen with synthetic devices such as
> > ones created by xen-kbdfront driver, which creates devices as being
> > capable of delivering all possible keys, since it doesn't know what
> > keys the backend may produce.
> >
> > To deal with such devices input core will attempt to trim key data,
> > in the hope that the rest of modalias string will fit in the given
> > buffer. When trimming key data it will indicate that it is not
> > complete by placing "+," sign, resulting in conversions like this:
> >
> > old: k71,72,73,74,78,7A,7B,7C,7D,8E,9E,A4,AD,E0,E1,E4,F8,174,
> > new: k71,72,73,74,78,7A,7B,7C,+,
> >
> > This should allow existing udev rules continue to work with existing
> > devices, and will also allow writing more complex rules that would
> > recognize trimmed modalias and check input device characteristics by
> > other means (for example by parsing KEY= data in uevent or parsing
> > input device sysfs attributes).
>
> I think adding these links may be useful for cross referencing:
> [1] https://github.com/systemd/systemd/issues/22944
> [2] https://lore.kernel.org/xen-devel/[email protected]/T/
>
> > Reported-by: Jason Andryuk <[email protected]>
> > Signed-off-by: Dmitry Torokhov <[email protected]>
>
> Thank you for looking at this. I think this idea of truncating the
> modalias is better than just dropping keys.
>
> cat-ing the individual sysfs files works, but there is still an issue:
>
> # sudo udevadm trigger --action=change
> [ 601.379977] ------------[ cut here ]------------
> [ 601.395959] add_uevent_var: buffer size too small
> [ 601.412009] WARNING: CPU: 0 PID: 630 at lib/kobject_uevent.c:671
> add_uevent_var+0x11c/0x130
..
> Another path needs to truncate the buffer? Or the problem is that the
> total uevent buffer size is what matters - not just the keys modalias?
Yes, exactly right - the driver core may add more environment variables,
such as SEQNUM=, HOME=, PATH=.
I created v2 of the patch that tries to leave some space at the end for
these additional variables.
>
> My other thought is wondering whether the presence of '+' will cause
> parsing errors? Has '+' been used before - or will it be an
> unexpected character?
I hope not. It is after "," and not a hex digit, so old code parsing
keys hopefully considers it as a separate part of modalias. I.e. it was
coping with:
...-e0,1,2,4,k110,111,112,113,114,115,r0,1,8,...
so I believe it should cope with
...-e0,1,2,4,k110,111,112,113,114,115,+,r0,1,8,...
But let's see Peter and Benjamin will say...
Thanks.
--
Dmitry