* [PATCH] acpi/nfit: Fix bus command validation @ 2019-02-07 23:57 Dan Williams 2019-02-08 0:41 ` Verma, Vishal L 2019-02-20 1:56 ` Jeff Moyer 0 siblings, 2 replies; 8+ messages in thread From: Dan Williams @ 2019-02-07 23:57 UTC (permalink / raw) To: linux-nvdimm Cc: stable, Vishal Verma, Grzegorz Burzynski, linux-kernel, vishal.l.verma 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(-) 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; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-07 23:57 [PATCH] acpi/nfit: Fix bus command validation Dan Williams @ 2019-02-08 0:41 ` Verma, Vishal L 2019-02-20 1:56 ` Jeff Moyer 1 sibling, 0 replies; 8+ messages in thread From: Verma, Vishal L @ 2019-02-08 0:41 UTC (permalink / raw) To: Williams, Dan J, linux-nvdimm Cc: linux-kernel, stable, Burzynski, Grzegorz, Verma, Vishal 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-07 23:57 [PATCH] acpi/nfit: Fix bus command validation Dan Williams 2019-02-08 0:41 ` Verma, Vishal L @ 2019-02-20 1:56 ` Jeff Moyer 2019-02-20 2:58 ` Dan Williams 1 sibling, 1 reply; 8+ messages in thread From: Jeff Moyer @ 2019-02-20 1:56 UTC (permalink / raw) To: Dan Williams; +Cc: linux-nvdimm, Vishal Verma, stable, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-20 1:56 ` Jeff Moyer @ 2019-02-20 2:58 ` Dan Williams 2019-02-20 8:29 ` Johannes Thumshirn 0 siblings, 1 reply; 8+ messages in thread From: Dan Williams @ 2019-02-20 2:58 UTC (permalink / raw) To: Jeff Moyer; +Cc: linux-nvdimm, Vishal Verma, stable, Linux Kernel Mailing List 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-20 2:58 ` Dan Williams @ 2019-02-20 8:29 ` Johannes Thumshirn 2019-02-20 16:15 ` Dan Williams 0 siblings, 1 reply; 8+ messages in thread From: Johannes Thumshirn @ 2019-02-20 8:29 UTC (permalink / raw) To: Dan Williams, Jeff Moyer Cc: Linux Kernel Mailing List, Vishal Verma, stable, linux-nvdimm 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 -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-20 8:29 ` Johannes Thumshirn @ 2019-02-20 16:15 ` Dan Williams 2019-02-20 17:21 ` Johannes Thumshirn 0 siblings, 1 reply; 8+ messages in thread From: Dan Williams @ 2019-02-20 16:15 UTC (permalink / raw) To: Johannes Thumshirn Cc: Jeff Moyer, Linux Kernel Mailing List, Vishal Verma, stable, linux-nvdimm 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-20 16:15 ` Dan Williams @ 2019-02-20 17:21 ` Johannes Thumshirn 2019-02-21 13:28 ` Johannes Thumshirn 0 siblings, 1 reply; 8+ messages in thread From: Johannes Thumshirn @ 2019-02-20 17:21 UTC (permalink / raw) To: Dan Williams Cc: Jeff Moyer, Linux Kernel Mailing List, Vishal Verma, stable, linux-nvdimm 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 -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] acpi/nfit: Fix bus command validation 2019-02-20 17:21 ` Johannes Thumshirn @ 2019-02-21 13:28 ` Johannes Thumshirn 0 siblings, 0 replies; 8+ messages in thread From: Johannes Thumshirn @ 2019-02-21 13:28 UTC (permalink / raw) To: Dan Williams Cc: Jeff Moyer, Linux Kernel Mailing List, Vishal Verma, linux-nvdimm, Dmitry Vyukov [-- Attachment #1: Type: text/plain, Size: 833 bytes --] [+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 -- Johannes Thumshirn SUSE Labs Filesystems jthumshirn@suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 [-- Attachment #2: dev_ndctl.txt --] [-- Type: text/plain, Size: 3090 bytes --] # Copyright 2019 syzkaller project authors. All rights reserved. # Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. #include <asm/ioctl.h> #include <linux/types.h> #include <uapi/linux/ndctl.h> resource fd_ndctl[fd] syz_open_dev$ndctl(dev ptr[in, string["/dev/ndctl#"]], id intptr, flags flags[open_flags]) fd_ndctl ioctl$ND_IOCTL_DIMM_FLAGS(fd fd_ndctl, cmd const[ND_IOCTL_DIMM_FLAGS], arg ptr[in, nd_cmd_dimm_flags]) ioctl$ND_IOCTL_GET_CONFIG_SIZE(fd fd_ndctl, cmd const[ND_IOCTL_GET_CONFIG_SIZE], arg ptr[in, nd_cmd_get_config_size]) ioctl$ND_IOCTL_GET_CONFIG_DATA(fd fd_ndctl, cmd const[ND_IOCTL_GET_CONFIG_DAT], arg ptr[in, nd_cmd_get_config_data_hdr]) ioctl$ND_IOCTL_SET_CONFIG_DATA(fd fd_ndctl, cmd const[ND_IOCTL_SET_CONFIG_DATA], arg ptr[in, nd_cmd_set_config_hdr]) ioctl$ND_IOCTL_VENDOR(fd fd_ndctl, cmd const[ND_IOCTL_VENDOR], arg ptr[in, nd_cmd_vendor_hdr]) ioctl$ND_IOCTL_ARS_CAP(fd fd_ndctl, cmd const[ND_IOCTL_ARS_CAP], arg ptr[in, nd_cmd_ars_cap]) ioctl$ND_IOCTL_ARS_START(fd fd_ndctl, cmd const[ND_IOCTL_ARS_START], arg ptr[in, nd_cmd_ars_start]) ioctl$ND_IOCTL_ARS_STATUS(fd fd_ndctl, cmd const[ND_IOCTL_ARS_STATUS], arg ptr[in, nd_cmd_ars_status]) ioctl$ND_IOCTL_CLEAR_ERROR(fd fd_ndctl, cmd const[ND_IOCTL_CLEAR_ERROR], arg ptr[in, nd_cmd_clear_error]) ioctl$ND_IOCTL_CALL(fd fd_ndctl, cmd const[ND_IOCTL_CALL], arg ptr[in, nd_cmd_pkg]) nd_cmd_dimm_flags { status int32 flags int32 } [packed] nd_cmd_get_config_size { status int32 config_size int32 max_xfer int32 } [packed] nd_cmd_get_config_data_hdr { in_offset int32 in_length len[out_buf, int32] status int32 out_buf ptr[out, array[int8] } [packed] struct nd_cmd_set_config_hdr { in_offset int32 in_length len[in_buf, int32] in_buf ptr[in, array[int8] } [packed] struct nd_cmd_vendor_hdr { opcode int32 in_length len[in_buf, int32] in_buf ptr[in, array[int8] } [packed] nd_cmd_ars_cap { address int64 length int64 status int32 max_ars_out int32 clear_err_unit int32 flags int16 reserved int16 } [packed] nd_cmd_ars_start { address int64 length int64 type int16 flags int8 reserved array[const[0, int8], 5] status int32 scrub_time int32 } [packed] type nd_ars_record { handle int32 reserved int32 err_address int64 length int64 } [packed] nd_cmd_ars_status { status int32 out_length int32 address int64 length int64 restart_address int64 restart_length int64 type int16 flags int16 num_records len[records, int32] records ptr[out, array[nd_ars_records] } [packed] nd_cmd_clear_error { address int64 length int64 status int32 reserved array[const[0, int8], 4] cleared int64 } [packed] nd_cmd_pkg { nd_family int64 nd_command int64 nd_size_in len[nd_payload, int32] nd_size_out int32 nd_reserved2 array[const[0, int32], 9] nd_fw_size int32 nd_payload ptr [in, array[int8]] } [packed] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-21 13:28 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-07 23:57 [PATCH] acpi/nfit: Fix bus command validation Dan Williams 2019-02-08 0:41 ` Verma, Vishal L 2019-02-20 1:56 ` Jeff Moyer 2019-02-20 2:58 ` Dan Williams 2019-02-20 8:29 ` Johannes Thumshirn 2019-02-20 16:15 ` Dan Williams 2019-02-20 17:21 ` Johannes Thumshirn 2019-02-21 13:28 ` Johannes Thumshirn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).