From: Jane Chu <jane.chu@oracle.com>
To: Dave Jiang <dave.jiang@intel.com>,
dan.j.williams@intel.com, vishal.l.verma@intel.com,
ira.weiny@intel.com, jmoyer@redhat.com,
linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state
Date: Mon, 3 Aug 2020 14:10:10 -0700 [thread overview]
Message-ID: <73f2eadf-3377-db62-ebd1-1eff99d4842e@oracle.com> (raw)
In-Reply-To: <cb8c1944-f72c-ecfa-bd3d-276f504542e1@intel.com>
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@intel.com>
Sure. I take it you meant to separate the typo fix from the change that
tests the OVERWRITE bit first?
Regards,
-jane
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: Jane Chu <jane.chu@oracle.com>
To: Dave Jiang <dave.jiang@intel.com>,
dan.j.williams@intel.com, vishal.l.verma@intel.com,
ira.weiny@intel.com, jmoyer@redhat.com,
linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state
Date: Mon, 3 Aug 2020 14:10:10 -0700 [thread overview]
Message-ID: <73f2eadf-3377-db62-ebd1-1eff99d4842e@oracle.com> (raw)
In-Reply-To: <cb8c1944-f72c-ecfa-bd3d-276f504542e1@intel.com>
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@intel.com>
Sure. I take it you meant to separate the typo fix from the change that
tests the OVERWRITE bit first?
Regards,
-jane
next prev parent reply other threads:[~2020-08-03 21:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-24 16:09 [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state Jane Chu
2020-07-24 16:09 ` Jane Chu
2020-07-24 16:09 ` [PATCH 2/2] libnvdimm/security: ensure sysfs poll thread woke up and fetch updated attr Jane Chu
2020-07-24 16:09 ` Jane Chu
2020-08-03 20:42 ` Dave Jiang
2020-08-03 20:42 ` Dave Jiang
2020-08-03 17:51 ` [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state Jane Chu
2020-08-03 20:41 ` Dave Jiang
2020-08-03 20:41 ` Dave Jiang
2020-08-03 21:10 ` Jane Chu [this message]
2020-08-03 21:10 ` Jane Chu
2020-08-03 21:26 ` Dave Jiang
2020-08-03 21:26 ` Dave Jiang
-- strict thread matches above, loose matches on Subject: below --
2020-07-24 4:10 [PATCH 2/2] libnvdimm/security: ensure sysfs poll thread woke up and fetch updated attr Jane Chu
2020-07-24 4:10 ` [PATCH 1/2] libnvdimm/security: 'security' attr never show 'overwrite' state Jane Chu
2020-07-24 4:10 ` Jane Chu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=73f2eadf-3377-db62-ebd1-1eff99d4842e@oracle.com \
--to=jane.chu@oracle.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=ira.weiny@intel.com \
--cc=jmoyer@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.