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: <47ea20d8-444b-4c72-97d3-b04a6918b121@baylibre.com>
Date: Wed, 20 Nov 2024 12:33:36 -0600
From: David Lechner <dlechner@...libre.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>, linux-input@...r.kernel.org
Cc: Erick Archer <erick.archer@...look.com>,
 Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex

On 9/3/24 11:30 PM, Dmitry Torokhov wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
>  drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
> index 682a29c27832..7ac0cfc3e786 100644
> --- a/drivers/input/joystick/db9.c
> +++ b/drivers/input/joystick/db9.c
> @@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
>  {
>  	struct db9 *db9 = input_get_drvdata(dev);
>  	struct parport *port = db9->pd->port;
> -	int err;
>  
> -	err = mutex_lock_interruptible(&db9->mutex);
> -	if (err)
> -		return err;
> -
> -	if (!db9->used++) {
> -		parport_claim(db9->pd);
> -		parport_write_data(port, 0xff);
> -		if (db9_modes[db9->mode].reverse) {
> -			parport_data_reverse(port);
> -			parport_write_control(port, DB9_NORMAL);
> +	scoped_guard(mutex_intr, &db9->mutex) {
> +		if (!db9->used++) {
> +			parport_claim(db9->pd);
> +			parport_write_data(port, 0xff);
> +			if (db9_modes[db9->mode].reverse) {
> +				parport_data_reverse(port);
> +				parport_write_control(port, DB9_NORMAL);
> +			}
> +			mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
>  		}
> -		mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> +
> +		return 0;
>  	}
>  
> -	mutex_unlock(&db9->mutex);
> -	return 0;
> +	return -EINTR;

This patch and any others like it are potentially introducing a bug.

>From inspecting the source code, it looks like
mutex_lock_interruptible() can return -EINTR, -EALREADY, or -EDEADLK.

Before this patch, the return value of mutex_lock_interruptible() was
passed to the caller. Now, the return value is reduced to pass/fail
and only -EINTR is returned on failure when the reason could have
been something else.



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ