Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1736844

Re: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check

From Andy Shevchenko <andy.shevchenko@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
Date 2017-09-21 18:50 +0200
Message-ID <uscPM-3sF-7@gated-at.bofh.it> (permalink)
References <usabf-1IE-3@gated-at.bofh.it> <usabh-1IE-39@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Sep 21, 2017 at 4:57 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> The Dell WMI descriptor check is used as an indication that WMI
> calls are safe to run both when used with the notification
> ASL/GUID pair as well as the SMBIOS calling ASL/GUID pair.
>
> As some code in dell-wmi-smbios is already a prerequisite for
> dell-wmi, move the code for performing the descriptor check into
> dell-wmi-smbios and let both drivers use it from there.

> +       desc_buffer = (u32 *)obj->buffer.pointer;
> +

> +       if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] != 0x494D5720)

I was thinking about strncmp() here for a full line, though decide not
to push it anyhow. Those IDs is binary data, can be anything and you
have comment of what is expected here.

But I think it would be nice to create a separate definitions and make
comments there.

> +               dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
> +                       8, desc_buffer);

%8ph ?

> +
> +       if (desc_buffer[2] != 0 && desc_buffer[2] != 1)
> +               dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
> +                       desc_buffer[2]);

%u ? u32 can't be negative and you basically allow it.

> +
> +       if (desc_buffer[3] != 4096)
> +               dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
> +                       desc_buffer[3]);

Ditto.


P.S. I noticed this all in old code, so, you can address my comments
in a separate patch if you find them useful.

-- 
With Best Regards,
Andy Shevchenko

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/12] Introduce support for Dell SMBIOS over WMI Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
  [PATCH 08/12] platform/x86: wmi: Cleanup exit routine in reverse order of init Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
  [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor GUID modalias Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    Re: [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor  GUID modalias Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
  [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI interface Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    Re: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI  interface Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:30 +0200
      RE: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI  interface <Mario.Limonciello@dell.com> - 2017-09-25 21:30 +0200
  [PATCH 12/12] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
  [PATCH 01/12] platform/x86: dell-wmi: label driver as handling notifications Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    Re: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling  notifications Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
      RE: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling  notifications <Mario.Limonciello@dell.com> - 2017-09-25 22:20 +0200
  [PATCH 10/12] platform/x86: wmi: destroy on cleanup rather than unregister Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
  [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    Re: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:50 +0200
      RE: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check <Mario.Limonciello@dell.com> - 2017-09-21 23:00 +0200
  [PATCH 09/12] platform/x86: wmi: create character devices when requested by drivers Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    Re: [PATCH 09/12] platform/x86: wmi: create character devices when  requested by drivers Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:50 +0200
      RE: [PATCH 09/12] platform/x86: wmi: create character devices when  requested by drivers <Mario.Limonciello@dell.com> - 2017-09-21 21:30 +0200
  [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:10 +0200
    Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:30 +0200
    Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition  to driver Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
  [PATCH 05/12] platform/x86: dell-smbios: rename to dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:10 +0200
  Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:20 +0200
    RE: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-25 18:40 +0200
      Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:50 +0200
        RE: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-25 21:30 +0200

csiph-web