linux-kernel - Re: [PATCH v3 1/1] Resolve WMI query failures on some devices

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:   Thu, 9 Jun 2022 11:49:31 -0500
From:   Jorge Lopez <jorgealtxwork@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Hans de Goede <hdegoede@...hat.com>, balalic.enver@...il.com,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mark Gross <markgross@...nel.org>
Subject: Re: [PATCH v3 1/1] Resolve WMI query failures on some devices

Awesome, thanks!

On Thu, Jun 9, 2022 at 11:30 AM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
>
> On Wed, Jun 8, 2022 at 11:32 PM Jorge Lopez <jorgealtxwork@...il.com> wrote:
> >
> > WMI queries fail on some devices where the ACPI method HWMC
> > unconditionally attempts to create Fields beyond the buffer
> > if the buffer is too small, this breaks essential features
> > such as power profiles:
> >
> >          CreateByteField (Arg1, 0x10, D008)
> >          CreateByteField (Arg1, 0x11, D009)
> >          CreateByteField (Arg1, 0x12, D010)
> >          CreateDWordField (Arg1, 0x10, D032)
> >          CreateField (Arg1, 0x80, 0x0400, D128)
> >
> > In cases where args->data had zero length, ACPI BIOS Error
> > (bug): AE_AML_BUFFER_LIMIT, Field [D008] at bit
> > offset/length 128/8 exceeds size of target Buffer (128 bits)
> > (20211217/dsopcode-198) was obtained.
> >
> > ACPI BIOS Error (bug): AE_AML_BUFFER_LIMIT, Field [D009] at bit
> > offset/length 136/8 exceeds size of target Buffer (136bits)
> > (20211217/dsopcode-198)
> >
> > The original code created a buffer size of 128 bytes regardless if
> > the WMI call required a smaller buffer or not.  This particular
> > behavior occurs in older BIOS and reproduced in OMEN laptops.  Newer
> > BIOS handles buffer sizes properly and meets the latest specification
> > requirements.  This is the reason why testing with a dynamically
> > allocated buffer did not uncover any failures with the test systems at
> > hand.
> >
> > This patch was tested on several OMEN, Elite, and Zbooks.  It was
> > confirmed the patch resolves HPWMI_FAN GET/SET calls in an OMEN
> > Laptop 15-ek0xxx.  No problems were reported when testing on several Elite
> > and Zbooks notebooks.
>
> Perfect!
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
>
> Also needs a Fixes tag:
>
> Fixes: 4b4967cbd268 ("platform/x86: hp-wmi: Changing bios_args.data to
> be dynamically allocated")
>
> (I believe Hans can help you and add it on the fly)
>
> > Signed-off-by: Jorge Lopez <jorge.lopez2@...com>
> >
> > ---
> > Based on the latest platform-drivers-x86.git/for-next
> > ---
> >  drivers/platform/x86/hp-wmi.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> > index 0e9a25b56e0e..d3540dd62d06 100644
> > --- a/drivers/platform/x86/hp-wmi.c
> > +++ b/drivers/platform/x86/hp-wmi.c
> > @@ -290,14 +290,16 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
> >         struct bios_return *bios_return;
> >         union acpi_object *obj = NULL;
> >         struct bios_args *args = NULL;
> > -       int mid, actual_outsize, ret;
> > +       int mid, actual_insize, actual_outsize;
> >         size_t bios_args_size;
> > +       int ret;
> >
> >         mid = encode_outsize_for_pvsz(outsize);
> >         if (WARN_ON(mid < 0))
> >                 return mid;
> >
> > -       bios_args_size = struct_size(args, data, insize);
> > +       actual_insize = max(insize, 128);
> > +       bios_args_size = struct_size(args, data, actual_insize);
> >         args = kmalloc(bios_args_size, GFP_KERNEL);
> >         if (!args)
> >                 return -ENOMEM;
> > --
> > 2.25.1
> >
>
>
> --
> With Best Regards,
> Andy Shevchenko