nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Dave Jiang <dave.jiang@intel.com>
Cc: alison.schofield@intel.com, keescook@chromium.org,
	linux-nvdimm@lists.01.org, ebiggers3@gmail.com,
	dhowells@redhat.com, keyrings@vger.kernel.org
Subject: Re: [PATCH v10 06/12] nfit/libnvdimm: add set passphrase support for Intel nvdimms
Date: Fri, 28 Sep 2018 09:45:54 +0100	[thread overview]
Message-ID: <26387.1538124354@warthog.procyon.org.uk> (raw)
In-Reply-To: <153799483810.71621.11639561453544624699.stgit@djiang5-desk3.ch.intel.com>

Dave Jiang <dave.jiang@intel.com> wrote:

> +	down_read(&key->sem);
> +	payload = key->payload.data[0];
> +	down_read(&user_key->sem);
> +	upayload = user_key->payload.data[0];

Personally, I would do both downs first and then deref both payloads.  The
compiler probably will be blocked from rearranging things to move the first
deref after the second down.  Ideally it or the cpu should be able to move
things into the critical section, but the cpu barriers available may well
preclude that.  That means that the compiler has to use a resource (stack/reg)
to stash the value across the second down.

> +	 * We don't need to release key->sem here because nvdimm_repalce_key

nvdimm_replace_key I presume.

> +	sscanf(buf, "%s %u %u", cmd, &old_key, &new_key);

You should check that sscanf() returned 3.

Also, since there's no size limitation here on the cmd string, if someone, for
example, writes a string of SEC_CMD_SIZE lots of 'a', sscanf() will write all
of them into cmd[] and follow that with a NUL char, thereby overrunning your
cmd[] buffer.  You need to either make the buffer one bigger or put a size
limit in sscanf string, e.g.:

	if (sscanf(buf, "%" __stringify(SEC_CMD_SIZE) "s %u %u", ...) == 3)

-~-
Other than that, I think this is mostly right.

David
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

  parent reply	other threads:[~2018-09-28  8:45 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-26 20:46 [PATCH v10 00/12] Adding security support for nvdimm Dave Jiang
2018-09-26 20:46 ` [PATCH v10 01/12] nfit: add support for Intel DSM 1.7 commands Dave Jiang
2018-09-26 20:46 ` [PATCH v10 02/12] libnvdimm: create keyring to store security keys Dave Jiang
2018-09-26 20:47 ` [PATCH v10 03/12] nfit/libnvdimm: store dimm id as a member to struct nvdimm Dave Jiang
2018-09-26 20:47 ` [PATCH v10 04/12] keys: export lookup_user_key to external users Dave Jiang
2018-09-26 20:47 ` [PATCH v10 05/12] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs Dave Jiang
2018-09-26 20:47 ` [PATCH v10 06/12] nfit/libnvdimm: add set passphrase support for Intel nvdimms Dave Jiang
2018-09-26 20:47 ` [PATCH v10 07/12] nfit/libnvdimm: add disable passphrase support to Intel nvdimm Dave Jiang
2018-09-26 20:47 ` [PATCH v10 08/12] nfit/libnvdimm: add freeze security " Dave Jiang
2018-09-26 20:47 ` [PATCH v10 09/12] nfit/libnvdimm: add support for issue secure erase DSM " Dave Jiang
2018-09-26 20:47 ` [PATCH v10 10/12] nfit_test: add context to dimm_dev for nfit_test Dave Jiang
2018-09-26 20:47 ` [PATCH v10 11/12] nfit_test: add test support for Intel nvdimm security DSMs Dave Jiang
2018-09-26 20:47 ` [PATCH v10 12/12] libnvdimm: add documentation for nvdimm security support Dave Jiang
2018-09-26 21:28 ` [PATCH v10 00/12] Adding security support for nvdimm Dan Williams
2018-09-28  8:17 ` [PATCH v10 02/12] libnvdimm: create keyring to store security keys David Howells
2018-09-28  8:24 ` [PATCH v10 05/12] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs David Howells
2018-09-28  8:45 ` David Howells [this message]
2018-09-28  9:28 ` [PATCH v10 06/12] nfit/libnvdimm: add set passphrase support for Intel nvdimms David Howells

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=26387.1538124354@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=ebiggers3@gmail.com \
    --cc=keescook@chromium.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --subject='Re: [PATCH v10 06/12] nfit/libnvdimm: add set passphrase support for Intel nvdimms' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).