linux-kernel - Re: [lvc-project] [PATCH] Input: trackpoint

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]

Date:   Tue, 14 Mar 2023 14:13:56 +0100
From:   Andi Shyti <andi.shyti@...nel.org>
To:     Igor Artemiev <Igor.A.Artemiev@...t.ru>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        lvc-project@...uxtesting.org
Subject: Re: [lvc-project] [PATCH] Input: trackpoint - remove unreachable code

Hi Igor,

On Tue, Mar 14, 2023 at 03:27:14PM +0300, Igor Artemiev wrote:
> The trackpoint_sync() function always returnd 0.
> And there is no need to check its result.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 2a924d71794c ("Input: trackpoint - only expose supported controls for Elan, ALPS and NXP")
> Signed-off-by: Igor Artemiev <Igor.A.Artemiev@...t.ru>

I don't think it requires the Fixes tag... it's not really
broken.

> ---
>  drivers/input/mouse/trackpoint.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 4a86b3e31d3b..561a4d2d81ff 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -386,9 +386,7 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
>  	was_reset = tp->variant_id == TP_VARIANT_IBM &&
>  		    trackpoint_power_on_reset(&psmouse->ps2dev) == 0;
>  
> -	error = trackpoint_sync(psmouse, was_reset);
> -	if (error)
> -		return error;
> +	trackpoint_sync(psmouse, was_reset);

what worries me here is that if this returns always '0' who tells
me that it will always return '0'?

One day someone might add an error return path and you would miss
it here.

Would it make sense to make the trackpoint_sync() a void function
as well?

Andi

>  
>  	return 0;
>  }
> -- 
> 2.30.2
>