linux-kernel - Re: [PATCH v3 4/7] acpi, nfit: Use bus_dsm

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, 29 Jun 2017 14:55:55 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Jerry Hoemann <Jerry.Hoemann@....com>
Cc:     "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/7] acpi, nfit: Use bus_dsm_mask for passthru

On Thu, Jun 29, 2017 at 2:47 PM, Jerry Hoemann <jerry.hoemann@....com> wrote:
> On Thu, Jun 29, 2017 at 02:35:14PM -0700, Dan Williams wrote:
>> On Thu, Jun 29, 2017 at 9:56 AM, Jerry Hoemann <jerry.hoemann@....com> wrote:
>> > Populate bus_dsm_mask and use it to filter dsm calls that user can
>> > make through the pass thru interface.
>> >
>> > Signed-off-by: Jerry Hoemann <jerry.hoemann@....com>
>> > ---
>> >  drivers/acpi/nfit/core.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> > index b46fca2..971002b 100644
>> > --- a/drivers/acpi/nfit/core.c
>> > +++ b/drivers/acpi/nfit/core.c
>> > @@ -253,6 +253,8 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
>> >                 cmd_name = nvdimm_bus_cmd_name(cmd);
>> >                 cmd_mask = nd_desc->cmd_mask;
>> >                 dsm_mask = cmd_mask;
>> > +               if (cmd == ND_CMD_CALL)
>> > +                       dsm_mask = nd_desc->bus_dsm_mask;
>> >                 desc = nd_cmd_bus_desc(cmd);
>> >                 uuid = to_nfit_uuid(NFIT_DEV_BUS);
>> >                 handle = adev->handle;
>> > @@ -1624,6 +1626,9 @@ static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc)
>> >                 if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i))
>> >                         set_bit(i, &nd_desc->cmd_mask);
>> >         set_bit(ND_CMD_CALL, &nd_desc->cmd_mask);
>> > +       for (i = 0; i < ND_CMD_CALL; i++)
>> > +               if (acpi_check_dsm(adev->handle, uuid, 1, 1ULL << i))
>> > +                       set_bit(i, &nd_desc->bus_dsm_mask);
>> >  }
>>
>> This loop checks for function 6 which is specified as reserved. Lets
>> explicitly test for the known good function numbers with something
>> like this:
>>
>> /* this should be private in drivers/acpi/nfit/nfit.h */
>> enum nfit_aux_cmds {
>>         NFIT_CMD_TRANSLATE_SPA = 5,
>>         NFIT_CMD_ARS_INJECT_SET = 7,
>>         NFIT_CMD_ARS_INJECT_CLEAR = 8,
>>         NFIT_CMD_ARS_INJECT_GET = 9,
>> };
>>
>> bus_dsm_mask =
>>         (1 << ND_CMD_ARS_CAP) |
>>         (1 << ND_CMD_ARS_START) |
>>         (1 << ND_CMD_ARS_STATUS) |
>>         (1 << ND_CMD_CLEAR_ERROR) |
>>         (1 << NFIT_CMD_TRANSLATE_SPA) |
>>         (1 << NFIT_CMD_ARS_INJECT_SET) |
>>         (1 << NFIT_CMD_ARS_INJECT_CLEAR) |
>>         (1 << NFIT_CMD_ARS_INJECT_GET);
>>
>> for_each_set_bit(i, &bus_dsm_mask...
>
>
>   I added the for_each_set_bit check in patch 7 of the series.

True, but in a patch series we shouldn't introduce a bug in one patch
and fix it later in the same series. Also, if patch7 goes away we
would need to fold that enabling in here.

Part of trying to parse what 0x3bf meant lead me to this, and I'm
wondering if we should do the same for the other magic vendor dsm_mask
constants, but that's a patch for another time.