On Mon, Apr 29, 2024 at 02:50:41PM -0700, Dmitry Torokhov 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).
>
> Note that the driver core may try adding more uevent environment
> variables once input core is done adding its own, so when forming
> modalias we can not use the entire available buffer, so we reduce
> it by somewhat an arbitrary amount (96 bytes).
>
> Reported-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>
> v2: do not use entire available buffer when formatting modalias, leave
> some space for driver core to add more data.
ftr, there's nothing in the projects I maintain that require those bits
of the modalias, so I'm good :) I'm not aware of any parsers that would
struggle with the + sign here. git grep of systemd doesn't show anything
either, so I think we're good.
Took me an embarrassingly long time to wrap my head around the code but
Reviewed-by: Peter Hutterer <peter.hutterer@xxxxxxxxx>
Cheers,
Peter
>
> drivers/input/input.c | 108 +++++++++++++++++++++++++++++++++++-------
> 1 file changed, 91 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index b04bcdeee557..045f4b62088a 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);
>
> @@ -1601,6 +1656,23 @@ static int input_add_uevent_bm_var(struct kobj_uevent_env *env,
> return 0;
> }
>
> +/*
> + * This is a pretty gross hack. When building uevent data the driver core
> + * may try adding more environment variables to kobj_uevent_env without
> + * telling us, so we have no idea how much of the buffer we can use to
> + * avoid overflows/-ENOMEM elsewhere. To work around this let's artificially
> + * reduce amount of memory we will use for the modalias environment variable.
> + *
> + * The potential additions are:
> + *
> + * SEQNUM=18446744073709551615 - (%llu - 28 bytes)
> + * HOME=/ (6 bytes)
> + * PATH=/sbin:/bin:/usr/sbin:/usr/bin (34 bytes)
> + *
> + * 68 bytes total. Allow extra buffer - 96 bytes
> + */
> +#define UEVENT_ENV_EXTRA_LEN 96
> +
> static int input_add_uevent_modalias_var(struct kobj_uevent_env *env,
> const struct input_dev *dev)
> {
> @@ -1610,9 +1682,11 @@ static int input_add_uevent_modalias_var(struct kobj_uevent_env *env,
> return -ENOMEM;
>
> len = input_print_modalias(&env->buf[env->buflen - 1],
> - sizeof(env->buf) - env->buflen,
> - dev, 0);
> - if (len >= (sizeof(env->buf) - env->buflen))
> + (int)sizeof(env->buf) - env->buflen -
> + UEVENT_ENV_EXTRA_LEN,
> + dev);
> + if (len >= ((int)sizeof(env->buf) - env->buflen -
> + UEVENT_ENV_EXTRA_LEN))
> return -ENOMEM;
>
> env->buflen += len;
> --
> 2.44.0.769.g3c40516874-goog
>
>
> --
> Dmitry
