On Mon, Jul 2, 2018 at 4:39 PM, Dave Jiang <[email protected]> wrote:
> Prepping the libnvdimm to support security management by adding a keyring
> in order to provide passphrase management through the kernel key management
> APIs.
>
> Signed-off-by: Dave Jiang <[email protected]>
> ---
> drivers/nvdimm/dimm.c | 90
> +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/libnvdimm.h | 5 +++
> 2 files changed, 95 insertions(+)
>
> diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
> index 6c8fb7590838..ee0c68efa82a 100644
> --- a/drivers/nvdimm/dimm.c
> +++ b/drivers/nvdimm/dimm.c
> @@ -18,9 +18,46 @@
> #include <linux/slab.h>
> #include <linux/mm.h>
> #include <linux/nd.h>
> +#include <linux/cred.h>
> +#include <keys/user-type.h>
> +#include <linux/key-type.h>
> +#include <linux/keyctl.h>
> #include "label.h"
> #include "nd.h"
>
> +const struct cred *nvdimm_cred;
I assume this can be marked static?
> +static int nvdimm_key_instantiate(struct key *key,
> + struct key_preparsed_payload *prep);
> +static void nvdimm_key_destroy(struct key *key);
> +
> +struct key_type nvdimm_key_type = {
> + .name = "nvdimm",
> + .instantiate = nvdimm_key_instantiate,
> + .destroy = nvdimm_key_destroy,
> + .describe = user_describe,
> + .def_datalen = NVDIMM_DEFAULT_PASSPHRASE_LEN * 2,
Perhaps a comment on why the "* 2"?
> +};
> +
> +static int nvdimm_key_instantiate(struct key *key,
> + struct key_preparsed_payload *prep)
> +{
> + char *payload;
> +
> + payload = kzalloc(nvdimm_key_type.def_datalen, GFP_KERNEL);
> + if (!payload)
> + return -ENOMEM;
> +
> + key->datalen = min(nvdimm_key_type.def_datalen, prep->datalen);
> + memcpy(payload, prep->data, key->datalen);
> + key->payload.data[0] = payload;
> + return 0;
> +}
> +
> +static void nvdimm_key_destroy(struct key *key)
> +{
> + kfree(key->payload.data[0]);
> +}
> +
The above appears to be generic infrastructure less associated with
the dimm driver and more the libnvdimm core. I think it should live in
drivers/nvdimm/dimm_devs.c.
> static int nvdimm_probe(struct device *dev)
> {
> struct nvdimm_drvdata *ndd;
> @@ -129,13 +166,66 @@ static struct nd_device_driver nvdimm_driver = {
> .type = ND_DRIVER_DIMM,
> };
>
> +static int nvdimm_register_keyring(void)
> +{
> + struct cred *cred;
> + struct key *keyring;
> + int rc;
> +
> + rc = register_key_type(&nvdimm_key_type);
> + if (rc < 0)
> + return rc;
> +
> + cred = prepare_kernel_cred(NULL);
> + if (!cred) {
> + rc = -ENOMEM;
> + goto failed_cred;
> + }
> +
> + keyring = keyring_alloc(".nvdimm",
> + GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
> + (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> + (KEY_USR_ALL & ~KEY_USR_SETATTR),
> + KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
> + if (IS_ERR(keyring)) {
> + rc = PTR_ERR(keyring);
> + goto failed_keyring;
> + }
> +
> + set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
> + cred->thread_keyring = keyring;
> + cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
> + nvdimm_cred = cred;
> + return 0;
> +
> + failed_cred:
> + unregister_key_type(&nvdimm_key_type);
> + failed_keyring:
> + put_cred(cred);
> + return rc;
> +}
> +
> +static void nvdimm_unregister_keyring(void)
> +{
> + key_revoke(nvdimm_cred->thread_keyring);
> + unregister_key_type(&nvdimm_key_type);
> + put_cred(nvdimm_cred);
> +}
> +
> int __init nvdimm_init(void)
> {
> + int rc;
> +
> + rc = nvdimm_register_keyring();
> + if (rc < 0)
> + return rc;
> +
> return nd_driver_register(&nvdimm_driver);
> }
>
> void nvdimm_exit(void)
> {
> + nvdimm_unregister_keyring();
> driver_unregister(&nvdimm_driver.drv);
> }
Yeah, the keyring registration infrastructure looks like details that
should be moved to drivers/nvdimm/dimm_devs.c. Would need to introduce
nvdimm_devs_init() and call it from libnvdimm_init(), but at least
nvdimm_devs_exit() already exists.
>
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index 472171af7f60..a250ff2a30df 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -155,6 +155,11 @@ static inline struct nd_blk_region_desc
> *to_blk_region_desc(
>
> }
>
> +extern struct key_type nvdimm_key_type;
> +
> +#define NVDIMM_DEFAULT_PASSPHRASE_LEN 32
> +#define NVDIMM_DEFAULT_DESC_LEN 32
Why the "_DEFAULT" designation? I.e. just shorten to
NVDIMM_PASSPHRASE_LEN and NVDIMM_KEY_DESC_LEN.
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
