Re: [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state

From: Jane Chu
Date: Mon Aug 03 2020 - 17:12:22 EST


Hi, Dave,

On 8/3/2020 1:41 PM, Dave Jiang wrote:
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@xxxxxxxxx>

Sure. I take it you meant to separate the typo fix from the change that tests the OVERWRITE bit first?

Regards,
-jane