Re: [PATCH] Input: try trimming too long modalias strings

[Dmitry Torokhov]

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
> <dmitry.torokhov@xxxxxxxxx> 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@xxxxxxxxxxxxxxxx/T/
> 
> > Reported-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> 
> 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