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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZjAY0uFxeglmMagh@google.com>
Date: Mon, 29 Apr 2024 15:01:54 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Jason Andryuk <jandryuk@...il.com>
Cc: linux-input@...r.kernel.org, Hans de Goede <hdegoede@...hat.com>,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	linux-kernel@...r.kernel.org,
	Peter Hutterer <peter.hutterer@...-t.net>
Subject: Re: [PATCH] Input: try trimming too long modalias strings

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@...il.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@...il.com>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.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

..

> 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