linux-kernel - Re: [PATCH v2 3/3] platform/x86: dell-wmi: add new dmi keys to bios_to_linux

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 8 Jun 2020 16:27:10 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Y Paritcher <y.linux@...itcher.com>
Cc:     linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Pali Rohár <pali@...nel.org>,
        Mario.Limonciello@...l.com
Subject: Re: [PATCH v2 3/3] platform/x86: dell-wmi: add new dmi keys to
 bios_to_linux_keycode

Hi--

On 6/8/20 4:05 PM, Y Paritcher wrote:
> Increase length of bios_to_linux_keycode to 2 bytes (the true size of a
> keycode) to allow for a new keycode 0xffff, this silences the following
> messages being logged at startup on a Dell Inspiron 5593:
> 
>     dell_wmi: firmware scancode 0x48 maps to unrecognized keycode 0xffff
>     dell_wmi: firmware scancode 0x50 maps to unrecognized keycode 0xffff
> 
> as per this code comment:
> 
>    Log if we find an entry in the DMI table that we don't
>    understand.  If this happens, we should figure out what
>    the entry means and add it to bios_to_linux_keycode.
> 
> These are keycodes included in the 0xB2 DMI table, for which the
> corosponding keys are not known.

  corresponding

> 
> Now when a user will encounter this key, a proper message wil be printed:
> 
>     dell_wmi: Unknown key with type 0xXXXX and code 0xXXXX pressed
> 
> This will then allow the key to be identified properly.
> 
> Signed-off-by: Y Paritcher <y.linux@...itcher.com>
> ---
>  drivers/platform/x86/dell-wmi.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 6b510f8431a3..dae1db96b5a0 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -196,7 +196,7 @@ struct dell_dmi_results {
>  };
>  
>  /* Uninitialized entries here are KEY_RESERVED == 0. */
> -static const u16 bios_to_linux_keycode[256] = {
> +static const u16 bios_to_linux_keycode[65536] = {

It surely seems odd to me to expand an array from 512 bytes to 128 Kbytes
just to handle one special case.  Can't it be handled in code as a
special case?

>  	[0]	= KEY_MEDIA,
>  	[1]	= KEY_NEXTSONG,
>  	[2]	= KEY_PLAYPAUSE,
> @@ -237,6 +237,7 @@ static const u16 bios_to_linux_keycode[256] = {
>  	[37]	= KEY_UNKNOWN,
>  	[38]	= KEY_MICMUTE,
>  	[255]	= KEY_PROG3,
> +	[65535]	= KEY_UNKNOWN,
>  };
>  
>  /*
> @@ -503,10 +504,7 @@ static void handle_dmi_entry(const struct dmi_header *dm, void *opaque)
>  					&table->keymap[i];
>  
>  		/* Uninitialized entries are 0 aka KEY_RESERVED. */
> -		u16 keycode = (bios_entry->keycode <
> -			       ARRAY_SIZE(bios_to_linux_keycode)) ?
> -			bios_to_linux_keycode[bios_entry->keycode] :
> -			KEY_RESERVED;
> +		u16 keycode = bios_to_linux_keycode[bios_entry->keycode];
>  
>  		/*
>  		 * Log if we find an entry in the DMI table that we don't
> 

Something like:

		u16 keycode;

		keycode = bios_entry->keycode == 0xffff ? KEY_UNKNOWN :
			(bios_entry->keycode <
			       ARRAY_SIZE(bios_to_linux_keycode)) ?
			bios_to_linux_keycode[bios_entry->keycode] :
			KEY_RESERVED;



Also please fix this:
(no To-header on input) <>

-- 
~Randy