diff mbox series

[v2] Input: xen-kbdfront - drop keys to shrink modalias

Message ID 20221209142615.33574-1-jandryuk@gmail.com
State Superseded
Headers show
Series [v2] Input: xen-kbdfront - drop keys to shrink modalias | expand

Commit Message

Jason Andryuk Dec. 9, 2022, 2:26 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.

An upper limit of KEY_KBD_LCD_MENU5 is still too large.  Use an upper
limit of KEY_BRIGHTNESS_MENU.

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

Empty space 0x28a..0x28f
KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad)
KEY_MACRO_RECORD_START          0x2b0
KEY_MACRO_RECORD_STOP           0x2b1
KEY_MACRO_PRESET_CYCLE          0x2b2
KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5)
Empty space 0x2b6..0x2b7
KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc)
Empty space 0x2bd..0x2bf
BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
Empty space 0x2e8..0x2ff

The modalias shrinks from 2082 to 1550 bytes.

A chunk of keys need to be removed to allow the keyboard to be used.
This may break some functionality, but the hope is these macro keys are
uncommon and don't affect any users.

[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(-)

v2:
Remove more keys: v1 didn't remove enough and modalias was still broken.

Comments

Mattijs Korpershoek Dec. 15, 2022, 1:54 p.m. UTC | #1
On Fri, Dec 09, 2022 at 09:26, 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.
>
> An upper limit of KEY_KBD_LCD_MENU5 is still too large.  Use an upper
> limit of KEY_BRIGHTNESS_MENU.
>
> This removes:
> BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
> Empty space 0x224..0x229
>
> Empty space 0x28a..0x28f
> KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad)
> KEY_MACRO_RECORD_START          0x2b0
> KEY_MACRO_RECORD_STOP           0x2b1
> KEY_MACRO_PRESET_CYCLE          0x2b2
> KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5)
> Empty space 0x2b6..0x2b7
> KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc)
> Empty space 0x2bd..0x2bf
> BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
> Empty space 0x2e8..0x2ff
>
> The modalias shrinks from 2082 to 1550 bytes.
>
> A chunk of keys need to be removed to allow the keyboard to be used.
> This may break some functionality, but the hope is these macro keys are
> uncommon and don't affect any users.
>
> [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(-)
>
> v2:
> Remove more keys: v1 didn't remove enough and modalias was still broken.
>
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index 8d8ebdc2039b..4ecb579df748 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.  There is a gap of buttons from
> +		 * BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
> +		 * defined. Then continue up to KEY_BRIGHTNESS_MENU as an upper
> +		 * limit. */
> +		for (i = KEY_OK; i < BTN_DPAD_UP; i++)
> +			__set_bit(i, kbd->keybit);
> +		for (i = KEY_ALS_TOGGLE; i <= KEY_BRIGHTNESS_MENU; i++)
>  			__set_bit(i, kbd->keybit);
>  
>  		ret = input_register_device(kbd);
> -- 
> 2.38.1
Jason Andryuk March 8, 2023, 4:26 p.m. UTC | #2
On Thu, Dec 15, 2022 at 8:54 AM Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
> On Fri, Dec 09, 2022 at 09:26, 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.
> >
> > An upper limit of KEY_KBD_LCD_MENU5 is still too large.  Use an upper
> > limit of KEY_BRIGHTNESS_MENU.
> >
> > This removes:
> > BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
> > Empty space 0x224..0x229
> >
> > Empty space 0x28a..0x28f
> > KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad)
> > KEY_MACRO_RECORD_START          0x2b0
> > KEY_MACRO_RECORD_STOP           0x2b1
> > KEY_MACRO_PRESET_CYCLE          0x2b2
> > KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5)
> > Empty space 0x2b6..0x2b7
> > KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc)
> > Empty space 0x2bd..0x2bf
> > BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
> > Empty space 0x2e8..0x2ff
> >
> > The modalias shrinks from 2082 to 1550 bytes.
> >
> > A chunk of keys need to be removed to allow the keyboard to be used.
> > This may break some functionality, but the hope is these macro keys are
> > uncommon and don't affect any users.
> >
> > [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>

Thank you, Mattjis.

Any other thoughts?  Can this patch be applied?

Thanks,
Jason
Jason Andryuk March 8, 2023, 4:27 p.m. UTC | #3
On Wed, Mar 8, 2023 at 11:26 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> On Thu, Dec 15, 2022 at 8:54 AM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
> >
> > On Fri, Dec 09, 2022 at 09:26, 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.
> > >
> > > An upper limit of KEY_KBD_LCD_MENU5 is still too large.  Use an upper
> > > limit of KEY_BRIGHTNESS_MENU.
> > >
> > > This removes:
> > > BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
> > > Empty space 0x224..0x229
> > >
> > > Empty space 0x28a..0x28f
> > > KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad)
> > > KEY_MACRO_RECORD_START          0x2b0
> > > KEY_MACRO_RECORD_STOP           0x2b1
> > > KEY_MACRO_PRESET_CYCLE          0x2b2
> > > KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5)
> > > Empty space 0x2b6..0x2b7
> > > KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc)
> > > Empty space 0x2bd..0x2bf
> > > BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
> > > Empty space 0x2e8..0x2ff
> > >
> > > The modalias shrinks from 2082 to 1550 bytes.
> > >
> > > A chunk of keys need to be removed to allow the keyboard to be used.
> > > This may break some functionality, but the hope is these macro keys are
> > > uncommon and don't affect any users.
> > >
> > > [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>
>
> Thank you, Mattjis.

Thank you, Mattijs.  My apologies.

Regards,
Jason
Mattijs Korpershoek March 9, 2023, 4:09 p.m. UTC | #4
Hi Jason,

On mer., mars 08, 2023 at 11:26, Jason Andryuk <jandryuk@gmail.com> wrote:

> On Thu, Dec 15, 2022 at 8:54 AM Mattijs Korpershoek
> <mkorpershoek@baylibre.com> wrote:
>>
>> On Fri, Dec 09, 2022 at 09:26, 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.
>> >
>> > An upper limit of KEY_KBD_LCD_MENU5 is still too large.  Use an upper
>> > limit of KEY_BRIGHTNESS_MENU.
>> >
>> > This removes:
>> > BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
>> > Empty space 0x224..0x229
>> >
>> > Empty space 0x28a..0x28f
>> > KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad)
>> > KEY_MACRO_RECORD_START          0x2b0
>> > KEY_MACRO_RECORD_STOP           0x2b1
>> > KEY_MACRO_PRESET_CYCLE          0x2b2
>> > KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5)
>> > Empty space 0x2b6..0x2b7
>> > KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc)
>> > Empty space 0x2bd..0x2bf
>> > BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
>> > Empty space 0x2e8..0x2ff
>> >
>> > The modalias shrinks from 2082 to 1550 bytes.
>> >
>> > A chunk of keys need to be removed to allow the keyboard to be used.
>> > This may break some functionality, but the hope is these macro keys are
>> > uncommon and don't affect any users.
>> >
>> > [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>
>
> Thank you, Mattjis.
>
> Any other thoughts?  Can this patch be applied?

That's not up to to decide, Dmitry might pick this up or give you a
review whenever he has time.

>
> Thanks,
> Jason
diff mbox series

Patch

diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 8d8ebdc2039b..4ecb579df748 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.  There is a gap of buttons from
+		 * BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
+		 * defined. Then continue up to KEY_BRIGHTNESS_MENU as an upper
+		 * limit. */
+		for (i = KEY_OK; i < BTN_DPAD_UP; i++)
+			__set_bit(i, kbd->keybit);
+		for (i = KEY_ALS_TOGGLE; i <= KEY_BRIGHTNESS_MENU; i++)
 			__set_bit(i, kbd->keybit);
 
 		ret = input_register_device(kbd);