linux-kernel - Re: [PATCH] acpi/nfit: Fix command-supported detection

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: <x49sgxvclmu.fsf@redhat.com>
Date:   Mon, 14 Jan 2019 10:19:21 -0500
From:   Jeff Moyer <jmoyer@...hat.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     linux-nvdimm@...ts.01.org, Sujith Pandel <sujith_pandel@...l.com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] acpi/nfit: Fix command-supported detection

Dan Williams <dan.j.williams@...el.com> writes:

> The _DSM function number validation only happens to succeed when the
> generic Linux command number translation corresponds with a
> DSM-family-specific function number. This breaks NVDIMM-N
> implementations that correctly implement _LSR, _LSW, and _LSI, but do
> not happen to publish support for DSM function numbers 4, 5, and 6.
>
> Recall that the support for _LS{I,R,W} family of methods results in the
> DIMM being marked as supporting those command numbers at
> acpi_nfit_register_dimms() time. The DSM function mask is only used for
> ND_CMD_CALL support of non-NVDIMM_FAMILY_INTEL devices.
>
> Fixes: 31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command...")
> Cc: <stable@...r.kernel.org>
> Link: https://github.com/pmem/ndctl/issues/78
> Reported-by: Sujith Pandel <sujith_pandel@...l.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> ---
> Sujith, this is a larger change than what you originally tested, but it
> should behave the same. I wanted to consolidate all the code that
> handles Linux command number to DIMM _DSM function number translation.
>
> If you have a chance to re-test with this it would be much appreciated.
>
> Thanks for the report!
>
>  drivers/acpi/nfit/core.c |   43 +++++++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 790691d9a982..d5d64e90ae71 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -409,6 +409,29 @@ static bool payload_dumpable(struct nvdimm *nvdimm, unsigned int func)
>  	return true;
>  }
>  
> +static int cmd_to_func(struct nvdimm *nvdimm, unsigned int cmd,
> +		struct nd_cmd_pkg *call_pkg)
> +{
> +	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);

Minor nit: Seems like the function could take an nfit_mem parameter instead of an nvdimm.

> +
> +	if (cmd == ND_CMD_CALL) {
> +		int i;
> +
> +		if (call_pkg && nfit_mem->family != call_pkg->nd_family)
> +			return -ENOTTY;
> +
> +		for (i = 0; i < ARRAY_SIZE(call_pkg->nd_reserved2); i++)
> +			if (call_pkg->nd_reserved2[i])
> +				return -EINVAL;
> +		return call_pkg->nd_command;
> +	}
> +
> +	/* Linux ND commands == NVDIMM_FAMILY_INTEL function numbers */
> +	if (nfit_mem->family == NVDIMM_FAMILY_INTEL)
> +		return cmd;
> +	return 0;

Function zero?  Is that really the right thing to return here?

Cheers,
Jeff