diff mbox series

Input: xen-kbdfront - drop keys to shrink modalias

Message ID 20221019201458.21803-1-jandryuk@gmail.com (mailing list archive)
State Superseded
Headers show
Series Input: xen-kbdfront - drop keys to shrink modalias | expand

Commit Message

Jason Andryuk Oct. 19, 2022, 8:14 p.m. UTC
xen kbdfront registers itself as being able to deliver *any* key since
it doesn't know what keys the backend may produce.

Unfortunately, the generated modalias gets too large and uevent creation
fails with -ENOMEM.

This can lead to gdm not using the keyboard since there is no seat
associated [1] and the debian installer crashing [2].

Trim the ranges of key capabilities by removing some BTN_* ranges.
While doing this, some neighboring undefined ranges are removed to trim
it further.

This removes:
BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
Empty space 0x224..0x229

Emtpy space 0x2bd..0x2bf
BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
Empty space 0x2e8..0x2ff

The modalias shrinks from 2082 to 1754 bytes.

[1] https://github.com/systemd/systemd/issues/22944
[2] https://lore.kernel.org/xen-devel/87o8dw52jc.fsf@vps.thesusis.net/T/

Cc: Phillip Susi <phill@thesusis.net>
Cc: stable@vger.kernel.org
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 drivers/input/misc/xen-kbdfront.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Mattijs Korpershoek Oct. 20, 2022, 8:31 a.m. UTC | #1
On Wed, Oct 19, 2022 at 16:14, Jason Andryuk <jandryuk@gmail.com> wrote:

> xen kbdfront registers itself as being able to deliver *any* key since
> it doesn't know what keys the backend may produce.
>
> Unfortunately, the generated modalias gets too large and uevent creation
> fails with -ENOMEM.
>
> This can lead to gdm not using the keyboard since there is no seat
> associated [1] and the debian installer crashing [2].
>
> Trim the ranges of key capabilities by removing some BTN_* ranges.
> While doing this, some neighboring undefined ranges are removed to trim
> it further.
>
> This removes:
> BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
> Empty space 0x224..0x229
>
> Emtpy space 0x2bd..0x2bf
> BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
> Empty space 0x2e8..0x2ff
>
> The modalias shrinks from 2082 to 1754 bytes.
>
> [1] https://github.com/systemd/systemd/issues/22944
> [2] https://lore.kernel.org/xen-devel/87o8dw52jc.fsf@vps.thesusis.net/T/
>
> Cc: Phillip Susi <phill@thesusis.net>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  drivers/input/misc/xen-kbdfront.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index 8d8ebdc2039b..23f37211be78 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -256,7 +256,14 @@ static int xenkbd_probe(struct xenbus_device *dev,
>  		__set_bit(EV_KEY, kbd->evbit);
>  		for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
>  			__set_bit(i, kbd->keybit);
> -		for (i = KEY_OK; i < KEY_MAX; i++)
> +		/* In theory we want to go KEY_OK..KEY_MAX, but that grows the
> +		 * modalias line too long.  KEY_KBD_LCD_MENU5 is the last
> +		 * defined non-button key. There is a gap of buttons from
> +		 * BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
> +		 * defined. */
> +		for (i = KEY_OK; i < BTN_DPAD_UP; i++)
> +			__set_bit(i, kbd->keybit);
> +		for (i = KEY_ALS_TOGGLE; i <= KEY_KBD_LCD_MENU5; i++)
>  			__set_bit(i, kbd->keybit);
>  
>  		ret = input_register_device(kbd);
> -- 
> 2.37.3
Jason Andryuk Oct. 20, 2022, 12:21 p.m. UTC | #2
On Thu, Oct 20, 2022 at 4:31 AM Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
> On Wed, Oct 19, 2022 at 16:14, Jason Andryuk <jandryuk@gmail.com> wrote:
>
> > xen kbdfront registers itself as being able to deliver *any* key since
> > it doesn't know what keys the backend may produce.
> >
> > Unfortunately, the generated modalias gets too large and uevent creation
> > fails with -ENOMEM.
> >
> > This can lead to gdm not using the keyboard since there is no seat
> > associated [1] and the debian installer crashing [2].
> >
> > Trim the ranges of key capabilities by removing some BTN_* ranges.
> > While doing this, some neighboring undefined ranges are removed to trim
> > it further.
> >
> > This removes:
> > BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
> > Empty space 0x224..0x229
> >
> > Emtpy space 0x2bd..0x2bf
> > BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
> > Empty space 0x2e8..0x2ff
> >
> > The modalias shrinks from 2082 to 1754 bytes.
> >
> > [1] https://github.com/systemd/systemd/issues/22944
> > [2] https://lore.kernel.org/xen-devel/87o8dw52jc.fsf@vps.thesusis.net/T/
> >
> > Cc: Phillip Susi <phill@thesusis.net>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

This patch doesn't work and needs to be withdrawn.  My gdm/udev
workaround was still active when I tested, so that is why I had a
working keyboard.  Sorry about that.

Now the question is, which additional keys can be omitted to trim the
modalias to an acceptable size?

Regards,
Jason

> > ---
> >  drivers/input/misc/xen-kbdfront.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> > index 8d8ebdc2039b..23f37211be78 100644
> > --- a/drivers/input/misc/xen-kbdfront.c
> > +++ b/drivers/input/misc/xen-kbdfront.c
> > @@ -256,7 +256,14 @@ static int xenkbd_probe(struct xenbus_device *dev,
> >               __set_bit(EV_KEY, kbd->evbit);
> >               for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> >                       __set_bit(i, kbd->keybit);
> > -             for (i = KEY_OK; i < KEY_MAX; i++)
> > +             /* In theory we want to go KEY_OK..KEY_MAX, but that grows the
> > +              * modalias line too long.  KEY_KBD_LCD_MENU5 is the last
> > +              * defined non-button key. There is a gap of buttons from
> > +              * BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
> > +              * defined. */
> > +             for (i = KEY_OK; i < BTN_DPAD_UP; i++)
> > +                     __set_bit(i, kbd->keybit);
> > +             for (i = KEY_ALS_TOGGLE; i <= KEY_KBD_LCD_MENU5; i++)
> >                       __set_bit(i, kbd->keybit);
> >
> >               ret = input_register_device(kbd);
> > --
> > 2.37.3
Jason Andryuk Oct. 20, 2022, 12:56 p.m. UTC | #3
On Thu, Oct 20, 2022 at 8:21 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> On Thu, Oct 20, 2022 at 4:31 AM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
> >
> > On Wed, Oct 19, 2022 at 16:14, Jason Andryuk <jandryuk@gmail.com> wrote:
> >
> > > xen kbdfront registers itself as being able to deliver *any* key since
> > > it doesn't know what keys the backend may produce.
> > >
> > > Unfortunately, the generated modalias gets too large and uevent creation
> > > fails with -ENOMEM.
> > >
> > > This can lead to gdm not using the keyboard since there is no seat
> > > associated [1] and the debian installer crashing [2].
> > >
> > > Trim the ranges of key capabilities by removing some BTN_* ranges.
> > > While doing this, some neighboring undefined ranges are removed to trim
> > > it further.
> > >
> > > This removes:
> > > BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
> > > Empty space 0x224..0x229
> > >
> > > Emtpy space 0x2bd..0x2bf
> > > BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
> > > Empty space 0x2e8..0x2ff
> > >
> > > The modalias shrinks from 2082 to 1754 bytes.
> > >
> > > [1] https://github.com/systemd/systemd/issues/22944
> > > [2] https://lore.kernel.org/xen-devel/87o8dw52jc.fsf@vps.thesusis.net/T/
> > >
> > > Cc: Phillip Susi <phill@thesusis.net>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> >
> > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>
> This patch doesn't work and needs to be withdrawn.  My gdm/udev
> workaround was still active when I tested, so that is why I had a
> working keyboard.  Sorry about that.
>
> Now the question is, which additional keys can be omitted to trim the
> modalias to an acceptable size?
>
> Regards,
> Jason
>
> > > ---
> > >  drivers/input/misc/xen-kbdfront.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> > > index 8d8ebdc2039b..23f37211be78 100644
> > > --- a/drivers/input/misc/xen-kbdfront.c
> > > +++ b/drivers/input/misc/xen-kbdfront.c
> > > @@ -256,7 +256,14 @@ static int xenkbd_probe(struct xenbus_device *dev,
> > >               __set_bit(EV_KEY, kbd->evbit);
> > >               for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> > >                       __set_bit(i, kbd->keybit);
> > > -             for (i = KEY_OK; i < KEY_MAX; i++)
> > > +             /* In theory we want to go KEY_OK..KEY_MAX, but that grows the
> > > +              * modalias line too long.  KEY_KBD_LCD_MENU5 is the last
> > > +              * defined non-button key. There is a gap of buttons from
> > > +              * BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
> > > +              * defined. */
> > > +             for (i = KEY_OK; i < BTN_DPAD_UP; i++)
> > > +                     __set_bit(i, kbd->keybit);
> > > +             for (i = KEY_ALS_TOGGLE; i <= KEY_KBD_LCD_MENU5; i++)

Changing the upper bound to KEY_BRIGHTNESS_MENU works.  That trims out
KEY_MACRO* and KEY_KBD_LCD_MENU*.

Something has to get trimmed out to bring the size down.  These are
probably less common and okay to remove, but I'm just guessing.

Regards,
Jason
diff mbox series

Patch

diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 8d8ebdc2039b..23f37211be78 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -256,7 +256,14 @@  static int xenkbd_probe(struct xenbus_device *dev,
 		__set_bit(EV_KEY, kbd->evbit);
 		for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
 			__set_bit(i, kbd->keybit);
-		for (i = KEY_OK; i < KEY_MAX; i++)
+		/* In theory we want to go KEY_OK..KEY_MAX, but that grows the
+		 * modalias line too long.  KEY_KBD_LCD_MENU5 is the last
+		 * defined non-button key. There is a gap of buttons from
+		 * BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
+		 * defined. */
+		for (i = KEY_OK; i < BTN_DPAD_UP; i++)
+			__set_bit(i, kbd->keybit);
+		for (i = KEY_ALS_TOGGLE; i <= KEY_KBD_LCD_MENU5; i++)
 			__set_bit(i, kbd->keybit);
 
 		ret = input_register_device(kbd);