linux-kernel - Re: [PATCH] platform/x86: asus-wmi: don't fail if platform

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: <34d15b62-42f2-4be3-9e0d-69f77fb52e90@app.fastmail.com>
Date: Tue, 10 Sep 2024 23:04:50 +1200
From: "Luke Jones" <luke@...nes.dev>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
 "Hans de Goede" <hdegoede@...hat.com>, corentin.chary@...il.com
Subject: Re: [PATCH] platform/x86: asus-wmi: don't fail if platform_profile already
 registered

On Tue, 10 Sep 2024, at 9:39 PM, Ilpo Järvinen wrote:
> On Tue, 10 Sep 2024, Luke D. Jones wrote:
> 
> > On some newer laptops ASUS laptops SPS support is advertised but not
> > actually used, causing the AMD driver to register as a platform_profile
> > handler.
> > 
> > If this happens then the asus_wmi driver would error with -EEXIST when
> > trying to register its own handler leaving the user with a possibly
> > unusable system. This is especially true for laptops with an MCU that emit
> > a stream of HID packets, some of which can be misinterpreted as shutdown
> > signals.
> > 
> > We can safely continue loading the driver instead of bombing out.
> > 
> > Signed-off-by: Luke D. Jones <luke@...nes.dev>
> > ---
> >  drivers/platform/x86/asus-wmi.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index fbb3345cc65a..d53c4aff519f 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -3876,8 +3876,13 @@ static int platform_profile_setup(struct asus_wmi *asus)
> >  asus->platform_profile_handler.choices);
> >  
> >  err = platform_profile_register(&asus->platform_profile_handler);
> > - if (err)
> > + if (err == -EEXIST) {
> > + pr_warn("%s, a platform_profile handler is already registered\n", __func__);
> > + return 0;
> > + } else if (err) {
> > + pr_err("%s, failed at platform_profile_register: %d\n", __func__, err);
> 
> Don't print __func__ in user visible warn/error/info messages but use 
> plain English please.

Ack. I've done this a few times now. Sorry I missed this one.

> 
> >  return err;
> > + }
> >  
> >  asus->platform_profile_support = true;
> >  return 0;
> > @@ -4752,7 +4757,7 @@ static int asus_wmi_add(struct platform_device *pdev)
> >  goto fail_fan_boost_mode;
> >  
> >  err = platform_profile_setup(asus);
> > - if (err)
> > + if (err && err != -EEXIST)
> 
> Hi Luke,
> 
> Hans had a comment about this line against the previous version.
> 
> Also, this patch has entirely lost the version information. It's v3 now I 
> think.

Uh.... sorry, I seem to have lost it in the depths of my inbox even with filters and searching. I'll look it up on lore and ensure everything is correct next go round.

Cheers,
Luke.