| Message ID | Zi2vDUZuVAEh4-yS@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | Input: try trimming too long modalias strings | expand |
Hi Dmitry, On Sat, Apr 27, 2024 at 10:06 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> 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/87o8dw52jc.fsf@vps.thesusis.net/T/ > Reported-by: Jason Andryuk <jandryuk@gmail.com> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> 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
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;
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 <jandryuk@gmail.com> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/input.c | 85 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 15 deletions(-)