linux-kernel - Re: [PATCH] platform/x86: dell-laptop: don't register platform::micmute if the related tokens don't exist.

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-next>] [day] [month] [year] [list]

Date:   Thu, 7 May 2020 13:45:17 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Koba Ko <koba.ko@...onical.com>
Cc:     Matthew Garrett <mjg59@...f.ucam.org>,
        Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: dell-laptop: don't register
 platform::micmute if the related tokens don't exist.

On Thursday 07 May 2020 19:27:47 Koba Ko wrote:
> Hi Pali,
> don't understand "registration and deregistration would be optional',
> could you explain more!?

After your patch led_classdev_register() function is not always called.
And led_classdev_unregister() should not be called when there is no
device registered.

> I will modify the comment of patch.
> 
> On Thu, May 7, 2020 at 7:13 PM Pali Rohár <pali@...nel.org> wrote:
> 
> > On Thursday 07 May 2020 17:42:42 koba.ko@...onical.com wrote:
> > > From: Koba Ko <koba.ko@...onical.com>
> > >
> > > Error messge is issued,
> > > "platform::micmute: Setting an LED's brightness failed (-19)",
> > > Even the device isn't presented.
> > >
> > > Get the related tokens of SMBIOS, GLOBAL_MIC_MUTE_DISABLE/ENABLE.
> > > If one of two tokens doesn't exist, don't register platform::micmute.
> > >
> > > Signed-off-by: Koba Ko <koba.ko@...onical.com>
> > > ---
> > >  drivers/platform/x86/dell-laptop.c | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/dell-laptop.c
> > b/drivers/platform/x86/dell-laptop.c
> > > index 1e46022fb2c5..afc1ded83e56 100644
> > > --- a/drivers/platform/x86/dell-laptop.c
> > > +++ b/drivers/platform/x86/dell-laptop.c
> > > @@ -2208,10 +2208,13 @@ static int __init dell_init(void)
> > >
> > >       dell_laptop_register_notifier(&dell_laptop_notifier);
> > >
> > > -     micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> > > -     ret = led_classdev_register(&platform_device->dev,
> > &micmute_led_cdev);
> > > -     if (ret < 0)
> > > -             goto fail_led;
> > > +     if (dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE) &&
> > > +         dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
> > > +             micmute_led_cdev.brightness =
> > ledtrig_audio_get(LED_AUDIO_MICMUTE);
> > > +             ret = led_classdev_register(&platform_device->dev,
> > &micmute_led_cdev);
> > > +             if (ret < 0)
> > > +                     goto fail_led;
> > > +     }
> >
> > Hello! I think that this is correct approach. Changing micmute LED is
> > done via those GLOBAL_MIC_MUTE_DISABLE and GLOBAL_MIC_MUTE_ENABLE
> > tokens. And if these tokens are not supported by hardware then linux
> > kernel should not register micmute LED device. There are lot of Dell
> > machines without led diode for microphone and these machines obviously
> > would not support those tokens.
> >
> > But this change is incomplete as registration of led class dev would be
> > optional. So deregistration also needs to be optional.
> >
> > And I think there is missing better description / explanation of this
> > change to make it clear what really happens.
> >
> > >
> > >       if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
> > >               return 0;
> > > --
> > > 2.17.1
> > >
> >