| Message ID | 165118817010.1772793.5101398830527716084.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bb7bf697fed58eae9d3445944e457ab0de4da54f |
| Headers | show |
| Series | nvdimm: Allow overwrite in the presence of disabled dimms | expand |
Dan Williams <dan.j.williams@intel.com> writes: > It is not clear why the original implementation of overwrite support > required the dimm driver to be active before overwrite could proceed. In Based on the log message, I'd say the intention was the opposite. > fact that can lead to cases where the kernel retains an invalid cached > copy of the labels from before the overwrite. Unfortunately the kernel > has not only allowed that case, but enforced it. > > Going forward, allow for overwrite to happen while the label area is > offline, and follow-on with updates to 'ndctl sanitize-dimm --overwrite' > to trigger the label area invalidation by default. That sounds reasonable to me. > Cc: Vishal Verma <vishal.l.verma@intel.com> > Cc: Dave Jiang <dave.jiang@intel.com> > Cc: Ira Weiny <ira.weiny@intel.com> > Cc: Jeff Moyer <jmoyer@redhat.com> > Reported-by: Krzysztof Kensicki <krzysztof.kensicki@intel.com> > Fixes: 7d988097c546 ("acpi/nfit, libnvdimm/security: Add security DSM overwrite support") > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/nvdimm/security.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c > index 4b80150e4afa..b5aa55c61461 100644 > --- a/drivers/nvdimm/security.c > +++ b/drivers/nvdimm/security.c > @@ -379,11 +379,6 @@ static int security_overwrite(struct nvdimm *nvdimm, unsigned int keyid) > || !nvdimm->sec.flags) > return -EOPNOTSUPP; > > - if (dev->driver == NULL) { > - dev_dbg(dev, "Unable to overwrite while DIMM active.\n"); > - return -EINVAL; > - } > - > rc = check_security_state(nvdimm); > if (rc) > return rc; Assuming you've tested this (please confirm): Acked-by: Jeff Moyer <jmoyer@redhat.com>
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c index 4b80150e4afa..b5aa55c61461 100644 --- a/drivers/nvdimm/security.c +++ b/drivers/nvdimm/security.c @@ -379,11 +379,6 @@ static int security_overwrite(struct nvdimm *nvdimm, unsigned int keyid) || !nvdimm->sec.flags) return -EOPNOTSUPP; - if (dev->driver == NULL) { - dev_dbg(dev, "Unable to overwrite while DIMM active.\n"); - return -EINVAL; - } - rc = check_security_state(nvdimm); if (rc) return rc;
It is not clear why the original implementation of overwrite support required the dimm driver to be active before overwrite could proceed. In fact that can lead to cases where the kernel retains an invalid cached copy of the labels from before the overwrite. Unfortunately the kernel has not only allowed that case, but enforced it. Going forward, allow for overwrite to happen while the label area is offline, and follow-on with updates to 'ndctl sanitize-dimm --overwrite' to trigger the label area invalidation by default. Cc: Vishal Verma <vishal.l.verma@intel.com> Cc: Dave Jiang <dave.jiang@intel.com> Cc: Ira Weiny <ira.weiny@intel.com> Cc: Jeff Moyer <jmoyer@redhat.com> Reported-by: Krzysztof Kensicki <krzysztof.kensicki@intel.com> Fixes: 7d988097c546 ("acpi/nfit, libnvdimm/security: Add security DSM overwrite support") Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/nvdimm/security.c | 5 ----- 1 file changed, 5 deletions(-)