| Message ID | 154749640259.63704.13504377094743676447.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ndctl: add security support | expand |
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 --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);
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(+)