linux-kernel - Re: [PATCH 2/7] Input: psmouse

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: <e8ad7e61-342a-8d53-7de1-f30a58c91975@suse.cz>
Date:   Mon, 25 Jun 2018 20:35:14 +0200
From:   Jiri Slaby <jslaby@...e.cz>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Lyude Paul <lyude@...hat.com>
Cc:     linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/7] Input: psmouse - clean up code

On 01/20/2018, 12:06 AM, Dmitry Torokhov wrote:
> - switch to using BIT() macros
> - use u8 instead of unsigned char for byte data
> - use input_set_capability() instead of manipulating capabilities bits
>   directly
> - use sign_extend32() when extracting wheel data.
> - do not abuse -1 as error code, propagate errors from various calls.
…
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
…
> @@ -157,39 +159,42 @@ psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse)
…
>  		case 0x00:
>  		case 0xC0:
> -			input_report_rel(dev, REL_WHEEL, (int) (packet[3] & 8) - (int) (packet[3] & 7));
> -			input_report_key(dev, BTN_SIDE, (packet[3] >> 4) & 1);
> -			input_report_key(dev, BTN_EXTRA, (packet[3] >> 5) & 1);
> +			input_report_rel(dev, REL_WHEEL,
> +					 -sign_extend32(packet[3], 3));
> +			input_report_key(dev, BTN_SIDE,  BIT(4));
> +			input_report_key(dev, BTN_EXTRA, BIT(5));
>  			break;
>  		}
>  		break;
>  
>  	case PSMOUSE_GENPS:
>  		/* Report scroll buttons on NetMice */
> -		input_report_rel(dev, REL_WHEEL, -(signed char) packet[3]);
> +		input_report_rel(dev, REL_WHEEL, -(s8) packet[3]);
>  
>  		/* Extra buttons on Genius NewNet 3D */
> -		input_report_key(dev, BTN_SIDE, (packet[0] >> 6) & 1);
> -		input_report_key(dev, BTN_EXTRA, (packet[0] >> 7) & 1);
> +		input_report_key(dev, BTN_SIDE,  BIT(6));
> +		input_report_key(dev, BTN_EXTRA, BIT(7));
>  		break;
>  
>  	case PSMOUSE_THINKPS:
>  		/* Extra button on ThinkingMouse */
> -		input_report_key(dev, BTN_EXTRA, (packet[0] >> 3) & 1);
> +		input_report_key(dev, BTN_EXTRA, BIT(3));

While hunting a 4.17 bug where some openSUSE users lost their ability to
mouse-click, I came across this commit. Putting aside it's a total mess
of multiple changes, were these changes above intentional at all? I mean
changing
  (packet[0] >> 3) & 1
to hardwired
  BIT(3)
does not look quite right. And if it is for some to me unknown reason,
it should have been properly documented in the commit log. I believe
your intent was:
  packet[0] & BIT(3)
?

thanks,
-- 
js
suse labs