| Message ID | 154958385172.3932544.193235520333886200.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | ebe9f6f19d80d8978d16078dff3d5bd93ad8d102 |
| Headers | show |
| Series | acpi/nfit: Fix bus command validation | expand |
On Thu, 2019-02-07 at 15:57 -0800, Dan Williams wrote: > Commit 11189c1089da "acpi/nfit: Fix command-supported detection" broke > ND_CMD_CALL for bus-level commands. The "func = cmd" assumption is only > valid for: > > ND_CMD_ARS_CAP > ND_CMD_ARS_START > ND_CMD_ARS_STATUS > ND_CMD_CLEAR_ERROR > > The function number otherwise needs to be pulled from the command > payload for: > > NFIT_CMD_TRANSLATE_SPA > NFIT_CMD_ARS_INJECT_SET > NFIT_CMD_ARS_INJECT_CLEAR > NFIT_CMD_ARS_INJECT_GET > > Update cmd_to_func() for the bus case and call it in the common path. > > Fixes: 11189c1089da ("acpi/nfit: Fix command-supported detection") > Cc: <stable@vger.kernel.org> > Cc: Vishal Verma <vishal.verma@intel.com> > Reported-by: Grzegorz Burzynski <grzegorz.burzynski@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/acpi/nfit/core.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) Looks good, Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 11189c1089da acpi/nfit: Fix command-supported detection.
The bot has tested the following trees: v4.20.7, v4.19.20, v4.14.98, v4.9.155.
v4.20.7: Build OK!
v4.19.20: Build OK!
v4.14.98: Build failed! Errors:
drivers/acpi/nfit/core.c:261:21: error: ‘nfit_mem’ undeclared (first use in this function)
v4.9.155: Failed to apply! Possible dependencies:
37d74841b9d4 ("acpi, nfit: Enable DSM pass thru for root functions.")
7db5bb33add5 ("libnvdimm, acpi, nfit: Add bus level dsm mask for pass thru.")
How should we proceed with this patch?
--
Thanks,
Sasha
Dan Williams <dan.j.williams@intel.com> writes: > Commit 11189c1089da "acpi/nfit: Fix command-supported detection" broke > ND_CMD_CALL for bus-level commands. The "func = cmd" assumption is only > valid for: > > ND_CMD_ARS_CAP > ND_CMD_ARS_START > ND_CMD_ARS_STATUS > ND_CMD_CLEAR_ERROR > > The function number otherwise needs to be pulled from the command > payload for: > > NFIT_CMD_TRANSLATE_SPA > NFIT_CMD_ARS_INJECT_SET > NFIT_CMD_ARS_INJECT_CLEAR > NFIT_CMD_ARS_INJECT_GET > > Update cmd_to_func() for the bus case and call it in the common path. > > Fixes: 11189c1089da ("acpi/nfit: Fix command-supported detection") > Cc: <stable@vger.kernel.org> > Cc: Vishal Verma <vishal.verma@intel.com> > Reported-by: Grzegorz Burzynski <grzegorz.burzynski@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Tricky code path, eh? Tested-by: Jeff Moyer <jmoyer@redhat.com> -Jeff > --- > drivers/acpi/nfit/core.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index e18ade5d74e9..c34c595d6bb0 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -415,7 +415,7 @@ static int cmd_to_func(struct nfit_mem *nfit_mem, unsigned int cmd, > if (call_pkg) { > int i; > > - if (nfit_mem->family != call_pkg->nd_family) > + if (nfit_mem && nfit_mem->family != call_pkg->nd_family) > return -ENOTTY; > > for (i = 0; i < ARRAY_SIZE(call_pkg->nd_reserved2); i++) > @@ -424,6 +424,10 @@ static int cmd_to_func(struct nfit_mem *nfit_mem, unsigned int cmd, > return call_pkg->nd_command; > } > > + /* In the !call_pkg case, bus commands == bus functions */ > + if (!nfit_mem) > + return cmd; > + > /* Linux ND commands == NVDIMM_FAMILY_INTEL function numbers */ > if (nfit_mem->family == NVDIMM_FAMILY_INTEL) > return cmd; > @@ -454,17 +458,18 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, > if (cmd_rc) > *cmd_rc = -EINVAL; > > + if (cmd == ND_CMD_CALL) > + call_pkg = buf; > + func = cmd_to_func(nfit_mem, cmd, call_pkg); > + if (func < 0) > + return func; > + > if (nvdimm) { > struct acpi_device *adev = nfit_mem->adev; > > if (!adev) > return -ENOTTY; > > - 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); > cmd_name = nvdimm_cmd_name(cmd); > cmd_mask = nvdimm_cmd_mask(nvdimm); > @@ -475,12 +480,9 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, > } else { > struct acpi_device *adev = to_acpi_dev(acpi_desc); > > - func = cmd; > 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; > + dsm_mask = nd_desc->bus_dsm_mask; > desc = nd_cmd_bus_desc(cmd); > guid = to_nfit_uuid(NFIT_DEV_BUS); > handle = adev->handle; > > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm
On Tue, Feb 19, 2019 at 5:57 PM Jeff Moyer <jmoyer@redhat.com> wrote: > > Dan Williams <dan.j.williams@intel.com> writes: > > > Commit 11189c1089da "acpi/nfit: Fix command-supported detection" broke > > ND_CMD_CALL for bus-level commands. The "func = cmd" assumption is only > > valid for: > > > > ND_CMD_ARS_CAP > > ND_CMD_ARS_START > > ND_CMD_ARS_STATUS > > ND_CMD_CLEAR_ERROR > > > > The function number otherwise needs to be pulled from the command > > payload for: > > > > NFIT_CMD_TRANSLATE_SPA > > NFIT_CMD_ARS_INJECT_SET > > NFIT_CMD_ARS_INJECT_CLEAR > > NFIT_CMD_ARS_INJECT_GET > > > > Update cmd_to_func() for the bus case and call it in the common path. > > > > Fixes: 11189c1089da ("acpi/nfit: Fix command-supported detection") > > Cc: <stable@vger.kernel.org> > > Cc: Vishal Verma <vishal.verma@intel.com> > > Reported-by: Grzegorz Burzynski <grzegorz.burzynski@intel.com> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > Tricky code path, eh? ioctl path, number one source of bugs / thrash in this subsystem. 2nd place, ARS. > Tested-by: Jeff Moyer <jmoyer@redhat.com> Thanks.
On 20/02/2019 03:58, Dan Williams wrote: [...] >> >> Tricky code path, eh? > > ioctl path, number one source of bugs / thrash in this subsystem. 2nd > place, ARS. Possibly unpopular idea, but should we maybe teach trinity/syzcaller about these ioctl()s? Better we find the bugs in a QA like environment than in the filed, I guess? Byte, Johannes
On Wed, Feb 20, 2019 at 12:30 AM Johannes Thumshirn <jthumshirn@suse.de> wrote: > > On 20/02/2019 03:58, Dan Williams wrote: > [...] > > >> > >> Tricky code path, eh? > > > > ioctl path, number one source of bugs / thrash in this subsystem. 2nd > > place, ARS. > > Possibly unpopular idea, but should we maybe teach trinity/syzcaller > about these ioctl()s? > > Better we find the bugs in a QA like environment than in the filed, I guess? I wouldn't be opposed to syzkaller fuzzing the nvdimm-ioctl path.
On 20/02/2019 17:15, Dan Williams wrote:> I wouldn't be opposed to syzkaller fuzzing the nvdimm-ioctl path. As a heads up, I've started adding the ioctl() definitions to syzcaller. Just so we don't duplicate any efforts. Byte, Johannes
[+CC dvyukov ] On 20/02/2019 18:21, Johannes Thumshirn wrote: > On 20/02/2019 17:15, Dan Williams wrote:> I wouldn't be opposed to > syzkaller fuzzing the nvdimm-ioctl path. > As a heads up, I've started adding the ioctl() definitions to syzcaller. > Just so we don't duplicate any efforts. So AFAICS this (see attachment) should do the trick. @dvyukov is there something I'm missing, or can syzkaller pick up the /dev/ndctl devices and start fuzzing the ioctl path with this? Thanks, Johannes
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index e18ade5d74e9..c34c595d6bb0 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -415,7 +415,7 @@ static int cmd_to_func(struct nfit_mem *nfit_mem, unsigned int cmd, if (call_pkg) { int i; - if (nfit_mem->family != call_pkg->nd_family) + if (nfit_mem && nfit_mem->family != call_pkg->nd_family) return -ENOTTY; for (i = 0; i < ARRAY_SIZE(call_pkg->nd_reserved2); i++) @@ -424,6 +424,10 @@ static int cmd_to_func(struct nfit_mem *nfit_mem, unsigned int cmd, return call_pkg->nd_command; } + /* In the !call_pkg case, bus commands == bus functions */ + if (!nfit_mem) + return cmd; + /* Linux ND commands == NVDIMM_FAMILY_INTEL function numbers */ if (nfit_mem->family == NVDIMM_FAMILY_INTEL) return cmd; @@ -454,17 +458,18 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, if (cmd_rc) *cmd_rc = -EINVAL; + if (cmd == ND_CMD_CALL) + call_pkg = buf; + func = cmd_to_func(nfit_mem, cmd, call_pkg); + if (func < 0) + return func; + if (nvdimm) { struct acpi_device *adev = nfit_mem->adev; if (!adev) return -ENOTTY; - 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); cmd_name = nvdimm_cmd_name(cmd); cmd_mask = nvdimm_cmd_mask(nvdimm); @@ -475,12 +480,9 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, } else { struct acpi_device *adev = to_acpi_dev(acpi_desc); - func = cmd; 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; + dsm_mask = nd_desc->bus_dsm_mask; desc = nd_cmd_bus_desc(cmd); guid = to_nfit_uuid(NFIT_DEV_BUS); handle = adev->handle;
Commit 11189c1089da "acpi/nfit: Fix command-supported detection" broke ND_CMD_CALL for bus-level commands. The "func = cmd" assumption is only valid for: ND_CMD_ARS_CAP ND_CMD_ARS_START ND_CMD_ARS_STATUS ND_CMD_CLEAR_ERROR The function number otherwise needs to be pulled from the command payload for: NFIT_CMD_TRANSLATE_SPA NFIT_CMD_ARS_INJECT_SET NFIT_CMD_ARS_INJECT_CLEAR NFIT_CMD_ARS_INJECT_GET Update cmd_to_func() for the bus case and call it in the common path. Fixes: 11189c1089da ("acpi/nfit: Fix command-supported detection") Cc: <stable@vger.kernel.org> Cc: Vishal Verma <vishal.verma@intel.com> Reported-by: Grzegorz Burzynski <grzegorz.burzynski@intel.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/acpi/nfit/core.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)