On Tue, Jan 15, 2019 at 2:46 PM Dan Williams dan.j.williams@intel.com wrote:
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@vger.kernel.org Link: https://github.com/pmem/ndctl/issues/78 Reported-by: Sujith Pandel sujith_pandel@dell.com Tested-by: Sujith Pandel sujith_pandel@dell.com Reviewed-by: Vishal Verma vishal.l.verma@intel.com Reviewed-by: Jeff Moyer jmoyer@redhat.com Signed-off-by: Dan Williams dan.j.williams@intel.com
drivers/acpi/nfit/core.c | 52 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 73281b19d3dd..9c95b82e5e5d 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -409,6 +409,32 @@ static bool payload_dumpable(struct nvdimm *nvdimm, unsigned int func) return true; }
+static int cmd_to_func(struct nfit_mem *nfit_mem, unsigned int cmd,
struct nd_cmd_pkg *call_pkg)+{
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;/** Force function number validation to fail since 0 is never* published as a valid function in dsm_mask.*/return 0;+}
int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc) { @@ -422,30 +448,21 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, unsigned long cmd_mask, dsm_mask; u32 offset, fw_status = 0; acpi_handle handle;
unsigned int func; const guid_t *guid;int rc, i;
int func, rc, i; if (cmd_rc) *cmd_rc = -EINVAL;
func = cmd;if (cmd == ND_CMD_CALL) {call_pkg = buf;
This breaks ND_CMD_CALL because now call_pkg is NULL for the rest of this routine. v4 inbound, as well as a backlog item to add an ND_CMD_CALL test-case to nfit_ctl_test(). The incremental fix is:
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 9c95b82e5e5d..71d03a4004fb 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -412,10 +412,10 @@ static bool payload_dumpable(struct nvdimm *nvdimm, unsigned int func) static int cmd_to_func(struct nfit_mem *nfit_mem, unsigned int cmd, struct nd_cmd_pkg *call_pkg) { - if (cmd == ND_CMD_CALL) { + if (call_pkg) { int i;
- if (call_pkg && nfit_mem->family != call_pkg->nd_family) + if (nfit_mem->family != call_pkg->nd_family) return -ENOTTY;
for (i = 0; i < ARRAY_SIZE(call_pkg->nd_reserved2); i++) @@ -460,7 +460,9 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, if (!adev) return -ENOTTY;
- func = cmd_to_func(nfit_mem, cmd, buf); + if (cmd == ND_CMD_CALL) + call_pkg = buf; + func = cmd_to_func(nfit_mem, cmd, call_pkg); if (func < 0) return func; dimm_name = nvdimm_name(nvdimm);