lkml.org 
[lkml]   [2015]   [Sep]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Incorrect key code parsing in dell-wmi.c since 5ea2559
On Saturday 04 July 2015 19:06:48 Pali Rohár wrote:
> Hello,
>
> I found another problem in dell-wmi.c code which is still partially in
> mainline kernel since commit 5ea2559726b786283236835dc2905c23b36ac91c:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5ea2559726b786283236835dc2905c23b36ac91c
>
> ===============================================
> commit 5ea2559726b786283236835dc2905c23b36ac91c
> Author: Rezwanul Kabir <Rezwanul_Kabir@dell.com>
> Date: Mon Nov 2 12:00:42 2009 -0500
>
> dell-wmi: Add support for new Dell systems
>
> Newer Dell systems support HotKey features differently from legacy
> systems. A new vendor specifc HotKey SMBIOS table (Type 0xB2) is
> defined. This table contains a mapping between scancode and the
> corresponding predefined keyfunction ( i.e. keycode).. Also, a new
> ACPI-WMI event type (called KeyIDList) with a value of 0x0010 is
> defined. Any BIOS containing 0xB2 table will send hotkey notifications
> using KeyIDList event.
>
> This is Rezwanul's patch, updated to ensure that brightness events are
> not sent if the backlight is controlled via ACPI and with the default
> keycode for the display output switching altered to match desktop
> expectations.
>
> Signed-off-by: Rezwanul Kabir <Rezwanul_Kabir@dell.com>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ===============================================
>
> Before this commit WMI event buffer was parsed as:
>
> int *buffer = (int *)obj->buffer.pointer;
> key = dell_wmi_get_entry_by_scancode(buffer[1] & 0xFFFF);
>
> So basically 4th-7th bytes are parsed.
>
> After this commit WMI event buffer is parsed as:
>
> u16 *buffer_entry = (u16 *)obj->buffer.pointer;
> reported_key = (int)buffer_entry[1] & 0xffff;
> key = dell_wmi_get_entry_by_scancode(reported_key);
>
> Which means that 2nd and 3rd bytes are parsed.
>
> Apparently this commit changed what kernel parse as keycode. And I bet
> this is some copy-paste error and not correct code. Variable buffer was
> changed from (int*) to (u16*) and which change could be "hidden" at time
> of code review.
>
> Next there is commit which somehow is trying to fix user problems:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d5164dbf1f651d1e955b158fb70a9c844cc91cd1
>
> ===============================================
> commit d5164dbf1f651d1e955b158fb70a9c844cc91cd1
> Author: Islam Amer <pharon@gmail.com>
> Date: Thu Jun 24 13:39:47 2010 -0400
>
> dell-wmi: Add support for eject key on Dell Studio 1555
>
> Fixes pressing the eject key on Dell Studio 1555 does not work and produces
> message :
>
> dell-wmi: Unknown key 0 pressed
>
> Signed-off-by: Islam Amer <pharon@gmail.com>
> ===============================================
>
> It changes parsing WMI event buffer to:
>
> u16 *buffer_entry = (u16 *)obj->buffer.pointer;
> if (buffer_entry[1] == 0x0)
> reported_key = (int)buffer_entry[2] & 0xffff;
> else
> reported_key = (int)buffer_entry[1] & 0xffff;
> key = dell_wmi_get_entry_by_scancode(reported_key);
>
> My idea is that Islam Amer tried to fix problem introduced in commit
> 5ea2559726b786283236835dc2905c23b36ac91c.
>
> According to some available very-very old Dell ACPI-WMI documentation at
>
> http://vpsservice1.sampo.com.tw/sampo_update/document/jimmy/ACPI-WMI%20.pdf
>
> and also from information from commit message 5ea2559 format of WMI is
> (u16*)[length, type, data, ...].
>
> Because type for hotkey is 0x0000 then it expected that kernel reported
> always key 0x0000...
>
> So I would propose to drop parsing "buffer_entry[1]" as key code and
> rewrite dell_wmi_notify function to always process WMI buffer as
> [length, type, data...].
>
> What do you think about it? It also simplify notify code as there is one
> branch for dell_new_hk_type and one for old parsing (with that entry[1]).
>

Hello! What do you think about it? Any objections?

--
Pali Rohár
pali.rohar@gmail.com


\
 
 \ /
  Last update: 2015-09-04 13:21    [W:0.056 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site