The APIs that iterate over the information contained in an ars_atatus
command require a prior, successfully completed ars_status command
struct. We were neglecting to verify that the firmware status too
indicates a success. Refactor this checking to mimic validate_ars_cap()
which checks both the command status as well as the formware status.

Reported-by: Tomasz Rochumski <[email protected]>
Cc: Dan Williams <[email protected]>
Signed-off-by: Vishal Verma <[email protected]>
---
 ndctl/lib/ars.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/ndctl/lib/ars.c b/ndctl/lib/ars.c
index 1ff6cf7..ef30617 100644
--- a/ndctl/lib/ars.c
+++ b/ndctl/lib/ars.c
@@ -216,15 +216,31 @@ NDCTL_EXPORT int ndctl_cmd_ars_in_progress(struct 
ndctl_cmd *cmd)
        return 0;
 }
 
+static bool __validate_ars_stat(struct ndctl_cmd *ars_stat)
+{
+       if (ars_stat->type != ND_CMD_ARS_STATUS || ars_stat->status != 0)
+               return false;
+       if ((*ars_stat->firmware_status & ARS_STATUS_MASK) != 0)
+               return false;
+       return true;
+}
+
+#define validate_ars_stat(ctx, ars_stat) \
+({ \
+       bool __valid = __validate_ars_stat(ars_stat); \
+       if (!__valid) \
+               dbg(ctx, "expected sucessfully completed ars_stat command\n"); \
+       __valid; \
+})
+
 NDCTL_EXPORT unsigned int ndctl_cmd_ars_num_records(struct ndctl_cmd *ars_stat)
 {
        struct ndctl_ctx *ctx = ndctl_bus_get_ctx(cmd_to_bus(ars_stat));
 
-       if (ars_stat->type == ND_CMD_ARS_STATUS && ars_stat->status == 0)
-               return ars_stat->ars_status->num_records;
+       if (!validate_ars_stat(ctx, ars_stat))
+               return 0;
 
-       dbg(ctx, "invalid ars_status\n");
-       return 0;
+       return ars_stat->ars_status->num_records;
 }
 
 NDCTL_EXPORT unsigned long long ndctl_cmd_ars_get_record_addr(
@@ -237,11 +253,10 @@ NDCTL_EXPORT unsigned long long 
ndctl_cmd_ars_get_record_addr(
                return 0;
        }
 
-       if (ars_stat->type == ND_CMD_ARS_STATUS && ars_stat->status == 0)
-               return ars_stat->ars_status->records[rec_index].err_address;
+       if (!validate_ars_stat(ctx, ars_stat))
+               return 0;
 
-       dbg(ctx, "invalid ars_status\n");
-       return 0;
+       return ars_stat->ars_status->records[rec_index].err_address;
 }
 
 NDCTL_EXPORT unsigned long long ndctl_cmd_ars_get_record_len(
@@ -254,11 +269,10 @@ NDCTL_EXPORT unsigned long long 
ndctl_cmd_ars_get_record_len(
                return 0;
        }
 
-       if (ars_stat->type == ND_CMD_ARS_STATUS && ars_stat->status == 0)
-               return ars_stat->ars_status->records[rec_index].length;
+       if (!validate_ars_stat(ctx, ars_stat))
+               return 0;
 
-       dbg(ctx, "invalid ars_status\n");
-       return 0;
+       return ars_stat->ars_status->records[rec_index].length;
 }
 
 NDCTL_EXPORT struct ndctl_cmd *ndctl_bus_cmd_new_clear_error(
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to