diff mbox series

[v8,01/12] ndctl: add support for display security state

Message ID 154749640259.63704.13504377094743676447.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State Superseded
Headers show
Series ndctl: add security support | expand

Commit Message

Dave Jiang Jan. 14, 2019, 8:06 p.m. UTC 
Adding libndctl API call for retrieving security state for a DIMM and also
adding support to ndctl list for displaying security state.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/ndctl-list.txt |    8 ++++++++
 ndctl/lib/dimm.c                   |   37 ++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym             |    5 +++++
 ndctl/libndctl.h                   |   13 +++++++++++++
 util/json.c                        |   31 ++++++++++++++++++++++++++++++
 5 files changed, 94 insertions(+)

Comments

Dan Williams Jan. 16, 2019, 1:13 a.m. UTC | #1 
Some comments below:

On Mon, Jan 14, 2019 at 12:07 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> Adding libndctl API call for retrieving security state for a DIMM and also
> adding support to ndctl list for displaying security state.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/ndctl/ndctl-list.txt |    8 ++++++++
>  ndctl/lib/dimm.c                   |   37 ++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym             |    5 +++++
>  ndctl/libndctl.h                   |   13 +++++++++++++
>  util/json.c                        |   31 ++++++++++++++++++++++++++++++
>  5 files changed, 94 insertions(+)
>
> diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
> index e24c8f40..bdd69add 100644
> --- a/Documentation/ndctl/ndctl-list.txt
> +++ b/Documentation/ndctl/ndctl-list.txt
> @@ -98,6 +98,14 @@ include::xable-region-options.txt[]
>  -D::
>  --dimms::
>         Include dimm info in the listing
> +[verse]
> +{
> +  "dev":"nmem0",
> +  "id":"cdab-0a-07e0-ffffffff",
> +  "handle":0,
> +  "phys_id":0,
> +  "security:":"disabled"
> +}
>
>  -H::
>  --health::
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index 79e2ca0a..e03135d9 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -587,3 +587,40 @@ NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
>
>         return strtoul(buf, NULL, 0);
>  }
> +
> +NDCTL_EXPORT int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
> +               enum nd_security_state *state)

Lets just have this routine return an "enum ndctl_security_state"
rather than an in/out parameter. Also name it "enum ndctl_" to match
the other enum definitions in libndctl.h, and use NDCTL_ in the enum
value names.

> +{
> +       struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +       char *path = dimm->dimm_buf;
> +       int len = dimm->buf_len;
> +       char buf[64];
> +       int rc;
> +
> +       if (snprintf(path, len, "%s/security", dimm->dimm_path) >= len) {
> +               err(ctx, "%s: buffer too small!\n",
> +                               ndctl_dimm_get_devname(dimm));
> +               return -ERANGE;
> +       }
> +
> +       rc = sysfs_read_attr(ctx, path, buf);
> +       if (rc < 0)
> +               return rc;
> +
> +       if (strcmp(buf, "unsupported") == 0)
> +               *state = ND_SECURITY_UNSUPPORTED;

We'll never get here because a missing "security" attribute is how the
kernel indicates "unsupported" and the sysfs_read_attr() call above
would have already failed with -ENOENT.

> +       else if (strcmp(buf, "disabled") == 0)
> +               *state = ND_SECURITY_DISABLED;
> +       else if (strcmp(buf, "unlocked") == 0)
> +               *state = ND_SECURITY_UNLOCKED;
> +       else if (strcmp(buf, "locked") == 0)
> +               *state = ND_SECURITY_LOCKED;
> +       else if (strcmp(buf, "frozen") == 0)
> +               *state = ND_SECURITY_FROZEN;
> +       else if (strcmp(buf, "overwrite") == 0)
> +               *state = ND_SECURITY_OVERWRITE;
> +       else
> +               *state = ND_SECURITY_INVALID;

An error will be thrown before we get here, no need for
ND_SECURITY_INVALID or ND_SECURITY_UNSUPPORTED should be defined.

> +
> +       return 0;
> +}
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 6c4c8b4d..1bd63fa1 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -385,3 +385,8 @@ global:
>         ndctl_namespace_get_next_badblock;
>         ndctl_dimm_get_dirty_shutdown;
>  } LIBNDCTL_17;
> +
> +LIBNDCTL_19 {
> +global:
> +       ndctl_dimm_get_security;
> +} LIBNDCTL_18;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index c81cc032..4255252c 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -681,6 +681,19 @@ enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
>  struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
>  int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
>
> +enum nd_security_state {
> +       ND_SECURITY_INVALID = -1,
> +       ND_SECURITY_UNSUPPORTED = 0,
> +       ND_SECURITY_DISABLED,
> +       ND_SECURITY_UNLOCKED,
> +       ND_SECURITY_LOCKED,
> +       ND_SECURITY_FROZEN,
> +       ND_SECURITY_OVERWRITE,
> +};
> +
> +int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
> +               enum nd_security_state *sstate);
> +
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif
> diff --git a/util/json.c b/util/json.c
> index 5c3424e2..e3b9e72e 100644
> --- a/util/json.c
> +++ b/util/json.c
> @@ -164,6 +164,7 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
>         unsigned int handle = ndctl_dimm_get_handle(dimm);
>         unsigned short phys_id = ndctl_dimm_get_phys_id(dimm);
>         struct json_object *jobj;
> +       enum nd_security_state sstate;
>
>         if (!jdimm)
>                 return NULL;
> @@ -243,6 +244,36 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
>                 json_object_object_add(jdimm, "flag_smart_event", jobj);
>         }
>
> +       if (ndctl_dimm_get_security(dimm, &sstate) == 0) {

Returning the state directly means we can lose a level of identation.

> +               switch (sstate) {
> +               case ND_SECURITY_UNSUPPORTED:
> +                       jobj = json_object_new_string("unsupported");
> +                       break;
> +               case ND_SECURITY_DISABLED:
> +                       jobj = json_object_new_string("disabled");
> +                       break;

It's less lines to write an if / else tree rather than a switch.

> +               case ND_SECURITY_UNLOCKED:
> +                       jobj = json_object_new_string("unlocked");
> +                       break;
> +               case ND_SECURITY_LOCKED:
> +                       jobj = json_object_new_string("locked");
> +                       break;
> +               case ND_SECURITY_FROZEN:
> +                       jobj = json_object_new_string("frozen");
> +                       break;
> +               case ND_SECURITY_OVERWRITE:
> +                       jobj = json_object_new_string("overwrite");
> +                       break;
> +               case ND_SECURITY_INVALID:
> +               default:
> +                       jobj = json_object_new_string("invalid");
> +                       break;
> +               }
> +               if (!jobj)
> +                       goto err;
> +               json_object_object_add(jdimm, "security", jobj);
> +       }
> +
diff mbox series

Patch

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index e24c8f40..bdd69add 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -98,6 +98,14 @@  include::xable-region-options.txt[]
 -D::
 --dimms::
 	Include dimm info in the listing
+[verse]
+{
+  "dev":"nmem0",
+  "id":"cdab-0a-07e0-ffffffff",
+  "handle":0,
+  "phys_id":0,
+  "security:":"disabled"
+}
 
 -H::
 --health::
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 79e2ca0a..e03135d9 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -587,3 +587,40 @@  NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
 
 	return strtoul(buf, NULL, 0);
 }
+
+NDCTL_EXPORT int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
+		enum nd_security_state *state)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char *path = dimm->dimm_buf;
+	int len = dimm->buf_len;
+	char buf[64];
+	int rc;
+
+	if (snprintf(path, len, "%s/security", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_dimm_get_devname(dimm));
+		return -ERANGE;
+	}
+
+	rc = sysfs_read_attr(ctx, path, buf);
+	if (rc < 0)
+		return rc;
+
+	if (strcmp(buf, "unsupported") == 0)
+		*state = ND_SECURITY_UNSUPPORTED;
+	else if (strcmp(buf, "disabled") == 0)
+		*state = ND_SECURITY_DISABLED;
+	else if (strcmp(buf, "unlocked") == 0)
+		*state = ND_SECURITY_UNLOCKED;
+	else if (strcmp(buf, "locked") == 0)
+		*state = ND_SECURITY_LOCKED;
+	else if (strcmp(buf, "frozen") == 0)
+		*state = ND_SECURITY_FROZEN;
+	else if (strcmp(buf, "overwrite") == 0)
+		*state = ND_SECURITY_OVERWRITE;
+	else
+		*state = ND_SECURITY_INVALID;
+
+	return 0;
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 6c4c8b4d..1bd63fa1 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -385,3 +385,8 @@  global:
 	ndctl_namespace_get_next_badblock;
 	ndctl_dimm_get_dirty_shutdown;
 } LIBNDCTL_17;
+
+LIBNDCTL_19 {
+global:
+	ndctl_dimm_get_security;
+} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index c81cc032..4255252c 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -681,6 +681,19 @@  enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
 struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
 int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
 
+enum nd_security_state {
+	ND_SECURITY_INVALID = -1,
+	ND_SECURITY_UNSUPPORTED = 0,
+	ND_SECURITY_DISABLED,
+	ND_SECURITY_UNLOCKED,
+	ND_SECURITY_LOCKED,
+	ND_SECURITY_FROZEN,
+	ND_SECURITY_OVERWRITE,
+};
+
+int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
+		enum nd_security_state *sstate);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/util/json.c b/util/json.c
index 5c3424e2..e3b9e72e 100644
--- a/util/json.c
+++ b/util/json.c
@@ -164,6 +164,7 @@  struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 	unsigned int handle = ndctl_dimm_get_handle(dimm);
 	unsigned short phys_id = ndctl_dimm_get_phys_id(dimm);
 	struct json_object *jobj;
+	enum nd_security_state sstate;
 
 	if (!jdimm)
 		return NULL;
@@ -243,6 +244,36 @@  struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 		json_object_object_add(jdimm, "flag_smart_event", jobj);
 	}
 
+	if (ndctl_dimm_get_security(dimm, &sstate) == 0) {
+		switch (sstate) {
+		case ND_SECURITY_UNSUPPORTED:
+			jobj = json_object_new_string("unsupported");
+			break;
+		case ND_SECURITY_DISABLED:
+			jobj = json_object_new_string("disabled");
+			break;
+		case ND_SECURITY_UNLOCKED:
+			jobj = json_object_new_string("unlocked");
+			break;
+		case ND_SECURITY_LOCKED:
+			jobj = json_object_new_string("locked");
+			break;
+		case ND_SECURITY_FROZEN:
+			jobj = json_object_new_string("frozen");
+			break;
+		case ND_SECURITY_OVERWRITE:
+			jobj = json_object_new_string("overwrite");
+			break;
+		case ND_SECURITY_INVALID:
+		default:
+			jobj = json_object_new_string("invalid");
+			break;
+		}
+		if (!jobj)
+			goto err;
+		json_object_object_add(jdimm, "security", jobj);
+	}
+
 	return jdimm;
  err:
 	json_object_put(jdimm);