Hi Peter, Christian,
On Sun, Oct 27, 2024 at 04:37:44PM +0100, Peter Seiderer wrote:
> Hello Ned, Christian, *,
>
> On Sun, 27 Oct 2024 15:06:09 +0100, Christian Heusel <christian@xxxxxxxxx> wrote:
>
> > On 24/10/26 07:15PM, Ned T. Crigler wrote:
> > > Hi,
> >
> > Hey Ned,
> >
> > > It looks like starting with kernel 6.11, disabling and re-enabling
> > > magic
> > > sysrq fails with these errors in dmesg:
> > >
> > > kernel: input: input_handler_check_methods: only one event processing
> > > method can be defined (sysrq)
> > > kernel: sysrq: Failed to register input handler, error -22
> > >
> > > after doing:
> > >
> > > # echo 0 > /proc/sys/kernel/sysrq
> > > # echo 438 > /proc/sys/kernel/sysrq
> > > # echo 0 > /proc/sys/kernel/sysrq
> > > # echo 438 > /proc/sys/kernel/sysrq
> > > # echo 0 > /proc/sys/kernel/sysrq
> > > # echo 438 > /proc/sys/kernel/sysrq
> >
> > I have found that this issue is also present in the latest mainline
> > release and bisected it to the following commit:
> >
> > d469647bafd9 ("Input: simplify event handling logic")
> >
>
> After the mentioned commit a call sysrq_register_handler() -->
> input_register_handler(&sysrq_handler) with sysrq_handler.filter set
> will result in sysrq_handler.events set to input_handler_events_filter,
> see drivers/input/input.c (line 2607 to 2608):
>
> 2596 int input_register_handler(struct input_handler *handler)
> 2597 {
> 2598 struct input_dev *dev;
> 2599 int error;
> 2600
> 2601 error = input_handler_check_methods(handler);
> 2602 if (error)
> 2603 return error;
> 2604
> 2605 INIT_LIST_HEAD(&handler->h_list);
> 2606
> 2607 if (handler->filter)
> 2608 handler->events = input_handler_events_filter;
> 2609 else if (handler->event)
> 2610 handler->events = input_handler_events_default;
> 2611 else if (!handler->events)
> 2612 handler->events = input_handler_events_null;
>
> So the second call will fail at the check 'input_handler_check_methods(handler)'
> which only allows one method to be set, see drivers/input/input.c:
>
> 2517 static int input_handler_check_methods(const struct input_handler *handler)
> 2518 {
> 2519 int count = 0;
> 2520
> 2521 if (handler->filter)
> 2522 count++;
> 2523 if (handler->events)
> 2524 count++;
> 2525 if (handler->event)
> 2526 count++;
> 2527
> 2528 if (count > 1) {
> 2529 pr_err("%s: only one event processing method can be defined (%s)\n",
> 2530 __func__, handler->name);
> 2531 return -EINVAL;
> 2532 }
> 2533
> 2534 return 0;
> 2535 }
>
>
> A quick fix/hack for the sysrq case:
>
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -1045,7 +1045,7 @@ static inline void sysrq_register_handler(void)
> int error;
>
> sysrq_of_get_keyreset_config();
> -
> + sysrq_handler.events = NULL;
> error = input_register_handler(&sysrq_handler);
> if (error)
> pr_err("Failed to register input handler, error %d", error);
> lines 1-13/13 (END)
>
> Regards,
> Peter
>
Thanks for tracking this down. It seems messy that the mentioned commit
changes input_register_handler to overwrite ->events for an internal purpose,
and callers may expect it to be unchanged, as sysrq does here by reusing
sysrq_handler.
--
Ned T. Crigler
>
> > The additional authors / maintainers have been added to the recipients
> > lists.
> >
> > I hope I didn't overlook any pending fixes.
> >
> > > --
> > > Ned T. Crigler
> >
> > Cheers,
> > Chris
>
