linux-kernel - Re: [PATCH 1/4] Input: make sure input handlers define only one processing method

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <cg35cp36opttnr2jgsqda2gsgqdn6vplc2pq3n3of3e356igua@vei6pdsw25si>
Date: Mon, 1 Jul 2024 09:36:08 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: linux-input@...r.kernel.org, Jeff LaBundy <jeff@...undy.com>, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] Input: make sure input handlers define only one
 processing method

Hi Dmitry,

On Jun 30 2024, Dmitry Torokhov wrote:
> Input core expects input handlers to be either filters, or regular
> handlers, but not both. Additionally, for regular handlers it does
> not make sense to define both single event method and batch method.
> 
> Refuse registering handler if it defines more than one method.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
>  drivers/input/input.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index fd4997ba263c..8434348faeac 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -2517,6 +2517,26 @@ void input_unregister_device(struct input_dev *dev)
>  }
>  EXPORT_SYMBOL(input_unregister_device);
>  
> +static int input_handler_check_methods(const struct input_handler *handler)
> +{
> +	int count = 0;
> +
> +	if (handler->filter)
> +		count++;
> +	if (handler->events)
> +		count++;
> +	if (handler->event)
> +		count++;
> +
> +	if (count != 1) {

Am I missing some upstream commit? I have the following:

in drivers/input/evdev.c:

static struct input_handler evdev_handler = {
	.event		= evdev_event,
	.events		= evdev_events,
	.connect	= evdev_connect,
	.disconnect	= evdev_disconnect,
	.legacy_minors	= true,
	.minor		= EVDEV_MINOR_BASE,
	.name		= "evdev",
	.id_table	= evdev_ids,
};

So here count should be 2 and evdev would be rejected?

And in drivers/tty/serial/kgdboc.c:

static struct input_handler kgdboc_reset_handler = {
	.connect	= kgdboc_reset_connect,
	.disconnect	= kgdboc_reset_disconnect,
	.name		= "kgdboc_reset",
	.id_table	= kgdboc_reset_ids,
};

here count would be 0 and kgdboc would also be rejected.

I agree on the intent of the patch, but these couple of input handlers
should be fixed if they are not already.

Cheers,
Benjamin

> +		pr_err("%s: only one event processing method should be defined (%s)\n",
> +		       __func__, handler->name);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * input_register_handler - register a new input handler
>   * @handler: handler to be registered
> @@ -2530,6 +2550,10 @@ int input_register_handler(struct input_handler *handler)
>  	struct input_dev *dev;
>  	int error;
>  
> +	error = input_handler_check_methods(handler);
> +	if (error)
> +		return error;
> +
>  	error = mutex_lock_interruptible(&input_mutex);
>  	if (error)
>  		return error;
> -- 
> 2.45.2.803.g4e1b14247a-goog
>