On 1/15/19 2:56 PM, Elliott, Robert (Persistent Memory) wrote:
>
>
>> -----Original Message-----
>> From: Linux-nvdimm [mailto:linux-nvdimm-boun...@lists.01.org] On Behalf Of
>> Dave Jiang
>> Sent: Thursday, December 13, 2018 5:49 PM
>> To: dan.j.willi...@intel.com
>> Cc: linux-nvdimm@lists.01.org
>> Subject: [PATCH v15 07/16] acpi/nfit, libnvdimm: Add unlock of nvdimm
>> support for Intel DIMMs
>>
>> From: Dan Williams <dan.j.willi...@intel.com>
>>
> ...
>> +static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
>> +{
>> +struct device *dev = &nvdimm->dev;
>> +struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
>> +struct key *key = NULL;
>> +int rc;
>> +
>> +/* The bus lock should be held at the top level of the call stack */
>> +lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
>> +
>> +if (!nvdimm->sec.ops || !nvdimm->sec.ops->unlock
>> +|| nvdimm->sec.state < 0)
>> +return -EIO;
>> +
>> +/*
>> + * If the pre-OS has unlocked the DIMM, attempt to send the key
>> + * from request_key() to the hardware for verification. Failure
>> + * to revalidate the key against the hardware results in a
>> + * freeze of the security configuration. I.e. if the OS does not
>> + * have the key, security is being managed pre-OS.
>> + */
>> +if (nvdimm->sec.state == NVDIMM_SECURITY_UNLOCKED) {
>> +if (!key_revalidate)
>> +return 0;
>> +
>> +key = nvdimm_key_revalidate(nvdimm);
>> +if (!key)
>> +return nvdimm_security_freeze(nvdimm);
>
> If it's already unlocked, regardless of whether nvdimm_key_revalidate()
> succeeds or fails, I'm not sure you should call nvdimm_security_freeze().
>
> The kernel doesn't know the key, so a hands-off policy seems prudent -
> don't send any commands that change the security state. Some other
> software (pre-OS or application) must be handling the feature, and
> sending freeze lock might interfere with that software.
The intention is that if we don't have the key or the key verification
fails, we want to prevent any additional interaction with security for
this particular boot session. At least for the Intel implementation, a
reboot will clear the freeze.
>
> Maybe you intended that to be "if (key)", intending to freeze if the
> kernel does have the correct key (so no application can change it and
> render the kernel out of sync)?
>
>
>> +} else
>> +key = nvdimm_request_key(nvdimm);
>> +
>> +if (!key)
>> +return -ENOKEY;
>> +
>> +rc = nvdimm->sec.ops->unlock(nvdimm, key_data(key));
>> +dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
>> +rc == 0 ? "success" : "fail");
>> +
>> +nvdimm_put_key(key);
>> +nvdimm->sec.state = nvdimm_security_state(nvdimm);
>> +return rc;
>> +}
>> +
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
