[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-keyrings
Subject:    Re: [PATCH v11 04/12] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
From:       Dan Williams <dan.j.williams () intel ! com>
Date:       2018-09-29 0:41:04
Message-ID: CAPcyv4irD-d=U9Y9hvyqg-6=8ST8dPnK1apviuOJ5bt2oX-Oew () mail ! gmail ! com
[Download RAW message or body]

On Fri, Sep 28, 2018 at 2:54 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> Add support to allow query the security status of the Intel nvdimms and
> also unlock the dimm via the kernel key management APIs. The passphrase is
> expected to be pulled from userspace through keyutils. Moving the Intel
> related bits to its own source file as well.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
[..]
> diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
> index 6c8fb7590838..b6381ddbd6c1 100644
> --- a/drivers/nvdimm/dimm.c
> +++ b/drivers/nvdimm/dimm.c
[..]
> @@ -25,6 +28,119 @@
>
>  static DEFINE_IDA(dimm_ida);
>
> +/*
> + * Find key that's cached with nvdimm
> + */
> +static struct key *nvdimm_get_key(struct device *dev)
> +{
> +       struct nvdimm *nvdimm = to_nvdimm(dev);
> +
> +       if (!nvdimm->key)
> +               return NULL;
> +
> +       if (key_validate(nvdimm->key) < 0)
> +               return NULL;
> +
> +       key_get(nvdimm->key);

I look at this and the double key_put() in nvdimm_release() and wonder
if this can be simplified by making the rule be that nvdimm->key, if
it is not NULL, is a valid key and the final put is done by
nvdimm_release(). Whenever the key becomes invalid that operation also
marks nvdimm->key NULL. This does mean we need some extra locking. but
we likely need that anyway because what's to stop the key going
invalid immediately after the key_validate() check above?
[prev in list] [next in list] [prev in thread] [next in thread]