lkml.org 
[lkml]   [2020]   [Aug]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state
From
Date


On 7/24/2020 9:09 AM, Jane Chu wrote:
> Since
> commit d78c620a2e82 ("libnvdimm/security: Introduce a 'frozen' attribute"),
> when issue
> # ndctl sanitize-dimm nmem0 --overwrite
> then immediately check the 'security' attribute,
> # cat /sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/nmem0/security
> unlocked
> Actually the attribute stays 'unlocked' through out the entire overwrite
> operation, never changed. That's because 'nvdimm->sec.flags' is a bitmap
> that has both bits set indicating 'overwrite' and 'unlocked'.
> But security_show() checks the mutually exclusive bits before it checks
> the 'overwrite' bit at last. The order should be reversed.
>
> The commit also has a typo: in one occasion, 'nvdimm->sec.ext_state'
> assignment is replaced with 'nvdimm->sec.flags' assignment for
> the NVDIMM_MASTER type.

May be best to split this fix to a different patch? Just thinking git bisect
later on to track issues. Otherwise Reviewed-by: Dave Jiang <dave.jiang@intel.com>

>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Fixes: d78c620a2e82 ("libnvdimm/security: Introduce a 'frozen' attribute")
> Signed-off-by: Jane Chu <jane.chu@oracle.com>
> ---
> drivers/nvdimm/dimm_devs.c | 4 ++--
> drivers/nvdimm/security.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index b7b77e8..5d72026 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -363,14 +363,14 @@ __weak ssize_t security_show(struct device *dev,
> {
> struct nvdimm *nvdimm = to_nvdimm(dev);
>
> + if (test_bit(NVDIMM_SECURITY_OVERWRITE, &nvdimm->sec.flags))
> + return sprintf(buf, "overwrite\n");
> if (test_bit(NVDIMM_SECURITY_DISABLED, &nvdimm->sec.flags))
> return sprintf(buf, "disabled\n");
> if (test_bit(NVDIMM_SECURITY_UNLOCKED, &nvdimm->sec.flags))
> return sprintf(buf, "unlocked\n");
> if (test_bit(NVDIMM_SECURITY_LOCKED, &nvdimm->sec.flags))
> return sprintf(buf, "locked\n");
> - if (test_bit(NVDIMM_SECURITY_OVERWRITE, &nvdimm->sec.flags))
> - return sprintf(buf, "overwrite\n");
> return -ENOTTY;
> }
>
> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
> index 4cef69b..8f3971c 100644
> --- a/drivers/nvdimm/security.c
> +++ b/drivers/nvdimm/security.c
> @@ -457,7 +457,7 @@ void __nvdimm_security_overwrite_query(struct nvdimm *nvdimm)
> clear_bit(NDD_WORK_PENDING, &nvdimm->flags);
> put_device(&nvdimm->dev);
> nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
> - nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_MASTER);
> + nvdimm->sec.ext_flags = nvdimm_security_flags(nvdimm, NVDIMM_MASTER);
> }
>
> void nvdimm_security_overwrite_query(struct work_struct *work)
>

\
 
 \ /
  Last update: 2020-08-03 22:42    [W:3.093 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site