linux-kernel - Re: [PATCH 2/2] 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] [day] [month] [year] [list]

Message-Id: <14b9f092-3118-4c71-8ad8-c40ac983abbe@app.fastmail.com>
Date: Tue, 06 Aug 2024 11:55:43 +1200
From: "Luke Jones" <luke@...nes.dev>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: platform-driver-x86@...r.kernel.org, corentin.chary@...il.com,
 "Hans de Goede" <hdegoede@...hat.com>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] platform/x86: asus-wmi: don't fail if platform_profile already
 registered

On Tue, 30 Jul 2024, at 3:45 AM, Ilpo Järvinen wrote:
> On Sat, 13 Jul 2024, Luke D. Jones wrote:
> 
> > On some newer laptops it appears that an AMD driver can register 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 4c129881ce28..7d87ff68f418 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -3836,8 +3836,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);
> 
> Please don't print __func__ to user in warnings or errors, and try to 
> write in English what is the reason (instead of resorting to use function 
> names).

Sorry, I do know this. The patch must have been submitted after your advice on it last time. Not sure.

In either case v2 incoming. I'll skip cover letter due to the tiny size.
 
> >  return err;
> > + }
> >  
> >  asus->platform_profile_support = true;
> >  return 0;
> > @@ -4713,7 +4718,7 @@ static int asus_wmi_add(struct platform_device *pdev)
> >  throttle_thermal_policy_set_default(asus);
> >  
> >  err = platform_profile_setup(asus);
> > - if (err)
> > + if (err && err != -EEXIST)
> >  goto fail_platform_profile_setup;
> >  
> >  err = asus_wmi_sysfs_init(asus->platform_device);
> > 
> 
>