linux-kernel - RE: [PATCH v4 0/3] platform/x86: dell-wmi: new keys

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:   Wed, 10 Jun 2020 19:22:56 +0000
From:   <Mario.Limonciello@...l.com>
To:     <andy@...radead.org>, <dvhart@...radead.org>
CC:     <linux-kernel@...r.kernel.org>,
        <platform-driver-x86@...r.kernel.org>, <mjg59@...f.ucam.org>,
        <y.linux@...itcher.com>, <pali@...nel.org>
Subject: RE: [PATCH v4 0/3] platform/x86: dell-wmi: new keys

> -----Original Message-----
> From: Y Paritcher <y.linux@...itcher.com>
> Sent: Wednesday, June 10, 2020 12:57 PM
> To: Pali Rohár
> Cc: linux-kernel@...r.kernel.org; platform-driver-x86@...r.kernel.org;
> Matthew Garrett; Limonciello, Mario
> Subject: [PATCH v4 0/3] platform/x86: dell-wmi: new keys
> 
> 
> [EXTERNAL EMAIL]
> 
> change since v3:
>     No code changes.
>     Update commit message to reflect info given by Mario at dell.
> 
> Is there anything more i have to do for the patches that were reviewed
> or will they be picked up by the maintainers?
> Thanks
> 
> Y Paritcher (3):
>   platform/x86: dell-wmi: add new backlight events
>   platform/x86: dell-wmi: add new keymap type 0x0012
>   platform/x86: dell-wmi: add new dmi mapping for keycode 0xffff
> 
>  drivers/platform/x86/dell-wmi.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> --
> 2.27.0

Andy,

The whole series looks good to me now.  You can put this on the patches
when they're swooped up.

Reviewed-by: Mario Limonciello <mario.limonciello@...l.com>

However I would like to note there was a comment that you had a direct question
asked by Pali that probably got lost in the thread.  This was on patch 3/3 on v3.
I think it's worth answering as it could dictate a follow up patch to change behavior.

The summary of my argument which led to his is nested somewhere in the thread was that
to most users this isn't useful since they can't act on it.  IE they can't use something
like setkeycodes and go on their merry way.  The user who could act on it by coming
to upstream and submitting questions and patches is more technical and having them
use dyndbg to turn on the messages about unknown shouldn't be a big deal.

> I'm not sure, but I thought that
> throwing warning or info message is the correct solution. Driver cannot
> handle something, so it inform about it, instead of silently dropping
> event. Same behavior I'm seeing in other kernel drivers.

> But looks like that you and Mario have opposite opinion, that kernel
> should not log unknown events and rather should drop them.

> I would like to have behavior of dell-wmi same as in other drivers for
> consistency, so the best would be to ask WMI/platform maintainers. They
> could have opinion how to handle these problem globally.

> ...

> Darren & Andy, could you please say something to this, what do you think
> about silently dropping events/actions which are currently unknown for
> dell-wmi driver? It is better to log them or not? Currently we are
> logging them.

Can you please advise which way you would rather have the subsystem go?