[PATCH] libnvdimm/dimm: Fix security capability detection for non-Intel NVDIMMs

[PATCH] libnvdimm/dimm: Fix security capability detection for non-Intel NVDIMMs

[Dan Williams]

Kees reports a crash with the following signature...

 RIP: 0010:nvdimm_visible+0x79/0x80
 [..]
 Call Trace:
  internal_create_group+0xf4/0x380
  sysfs_create_groups+0x46/0xb0
  device_add+0x331/0x680
  nd_async_device_register+0x15/0x60
  async_run_entry_fn+0x38/0x100

...when starting a QEMU environment with "label-less" DIMM. Without
labels QEMU does not publish any DSM methods. Without defined methods
the NVDIMM_FAMILY type is not established and the nfit driver will skip
registering security operations.

In that case the security state should be initialized to a negative
value in __nvdimm_create() and nvdimm_visible() should skip
interrogating the specific ops. However, since 'enum
nvdimm_security_state' was only defined to contain positive values the
"if (nvdimm->sec.state < 0)" check always fails.

Define a negative error state to allow negative state values to be
handled as expected.

Fixes: f2989396553a ("acpi/nfit, libnvdimm: Introduce nvdimm_security_ops")
Cc: Dave Jiang <dave.jiang@intel.com>
Reported-by: Kees Cook <keescook@chromium.org>
Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/libnvdimm.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 5440f11b0907..7315977b64da 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -160,6 +160,7 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
 }
 
 enum nvdimm_security_state {
+	NVDIMM_SECURITY_ERROR = -1,
 	NVDIMM_SECURITY_DISABLED,
 	NVDIMM_SECURITY_UNLOCKED,
 	NVDIMM_SECURITY_LOCKED,

Re: [PATCH] libnvdimm/dimm: Fix security capability detection for non-Intel NVDIMMs

[Dave Jiang]

On 1/8/19 6:24 PM, Dan Williams wrote:
> Kees reports a crash with the following signature...
> 
>  RIP: 0010:nvdimm_visible+0x79/0x80
>  [..]
>  Call Trace:
>   internal_create_group+0xf4/0x380
>   sysfs_create_groups+0x46/0xb0
>   device_add+0x331/0x680
>   nd_async_device_register+0x15/0x60
>   async_run_entry_fn+0x38/0x100
> 
> ...when starting a QEMU environment with "label-less" DIMM. Without
> labels QEMU does not publish any DSM methods. Without defined methods
> the NVDIMM_FAMILY type is not established and the nfit driver will skip
> registering security operations.
> 
> In that case the security state should be initialized to a negative
> value in __nvdimm_create() and nvdimm_visible() should skip
> interrogating the specific ops. However, since 'enum
> nvdimm_security_state' was only defined to contain positive values the
> "if (nvdimm->sec.state < 0)" check always fails.
> 
> Define a negative error state to allow negative state values to be
> handled as expected.
> 
> Fixes: f2989396553a ("acpi/nfit, libnvdimm: Introduce nvdimm_security_ops")
> Cc: Dave Jiang <dave.jiang@intel.com>
> Reported-by: Kees Cook <keescook@chromium.org>
> Tested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  include/linux/libnvdimm.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index 5440f11b0907..7315977b64da 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -160,6 +160,7 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
>  }
>  
>  enum nvdimm_security_state {
> +	NVDIMM_SECURITY_ERROR = -1,
>  	NVDIMM_SECURITY_DISABLED,
>  	NVDIMM_SECURITY_UNLOCKED,
>  	NVDIMM_SECURITY_LOCKED,
>