[ndctl PATCH 0/5] ndctl: misc static analysis fixes

[ndctl PATCH 0/5] ndctl: misc static analysis fixes

[Vishal Verma]

Fix up various static analysis reports from recent (and not-so-recent)
patches.

Vishal Verma (5):
  ndctl, inject: fix a resource leak in ndctl_namespace_get_clear_unit
  ndctl: fix a resource leak in submit_get_firmware_info
  libndctl: fix a resource leak in ndctl_dimm_get_{{event_}flags,
    health}
  ndctl, test: fix a potential null pointer dereference in 'ndctl test'
  ndctl, test: fix a resource leak in check_smart_threshold

 ndctl/dimm.c         | 17 ++++++++++-------
 ndctl/lib/inject.c   | 12 +++++++-----
 ndctl/lib/libndctl.c |  3 +++
 ndctl/test.c         |  2 ++
 test/libndctl.c      |  1 +
 5 files changed, 23 insertions(+), 12 deletions(-)

-- 
2.14.4

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

[ndctl PATCH 1/5] ndctl, inject: fix a resource leak in ndctl_namespace_get_clear_unit

[Vishal Verma]

Static analysis reports that we leak ndctl_cmd in the above function.
Fix by adding a proper cleanup path.

Fixes: 7271760ce96c ("libndctl, inject: inject fewer bytes per block by default")
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/lib/inject.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
index f9da02d..268c5cd 100644
--- a/ndctl/lib/inject.c
+++ b/ndctl/lib/inject.c
@@ -102,18 +102,20 @@ static int ndctl_namespace_get_clear_unit(struct ndctl_namespace *ndns)
 		&ns_size);
 	cmd = ndctl_bus_cmd_new_ars_cap(bus, ns_offset, ns_size);
 	rc = ndctl_cmd_submit(cmd);
-	if (rc) {
+	if (rc < 0) {
 		dbg(ctx, "Error submitting ars_cap: %d\n", rc);
-		return rc;
+		goto out;
 	}
 	clear_unit = ndctl_cmd_ars_cap_get_clear_unit(cmd);
 	if (clear_unit == 0) {
 		dbg(ctx, "Got an invalid clear_err_unit from ars_cap\n");
-		return -EINVAL;
+		rc = -EINVAL;
+		goto out;
 	}
-
+	rc = clear_unit;
+out:
 	ndctl_cmd_unref(cmd);
-	return clear_unit;
+	return rc;
 }
 
 static int ndctl_namespace_inject_one_error(struct ndctl_namespace *ndns,
-- 
2.14.4

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

[ndctl PATCH 2/5] ndctl: fix a resource leak in submit_get_firmware_info

[Vishal Verma]

Static analysis reports that we leak ndctl_cmd in the above function.
Fix by adding a proper cleanup path.

Cc: Dave Jiang <dave.jiang@intel.com>
Fixes: f86369ea29e2 ("ndctl: merge firmware-update into dimm.c as another dimm operation")
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/dimm.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 97643a3..a4203f3 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -428,36 +428,39 @@ static int submit_get_firmware_info(struct ndctl_dimm *dimm,
 
 	rc = ndctl_cmd_submit(cmd);
 	if (rc < 0)
-		return rc;
+		goto out;
 
+	rc = -ENXIO;
 	status = ndctl_cmd_fw_xlat_firmware_status(cmd);
 	if (status != FW_SUCCESS) {
 		fprintf(stderr, "GET FIRMWARE INFO on DIMM %s failed: %#x\n",
 				ndctl_dimm_get_devname(dimm), status);
-		return -ENXIO;
+		goto out;
 	}
 
 	fw->store_size = ndctl_cmd_fw_info_get_storage_size(cmd);
 	if (fw->store_size == UINT_MAX)
-		return -ENXIO;
+		goto out;
 
 	fw->update_size = ndctl_cmd_fw_info_get_max_send_len(cmd);
 	if (fw->update_size == UINT_MAX)
-		return -ENXIO;
+		goto out;
 
 	fw->query_interval = ndctl_cmd_fw_info_get_query_interval(cmd);
 	if (fw->query_interval == UINT_MAX)
-		return -ENXIO;
+		goto out;
 
 	fw->max_query = ndctl_cmd_fw_info_get_max_query_time(cmd);
 	if (fw->max_query == UINT_MAX)
-		return -ENXIO;
+		goto out;
 
 	fw->run_version = ndctl_cmd_fw_info_get_run_version(cmd);
 	if (fw->run_version == ULLONG_MAX)
-		return -ENXIO;
+		goto out;
 
 	rc = verify_fw_size(uctx);
+
+out:
 	ndctl_cmd_unref(cmd);
 	return rc;
 }
-- 
2.14.4

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

[ndctl PATCH 3/5] libndctl: fix a resource leak in ndctl_dimm_get_{{event_}flags, health}

[Vishal Verma]

Static analysis reports that we leak ndctl_cmd in the above functions.
Fix by adding proper cleanup paths.

Cc: QI Fuli <qi.fuli@jp.fujitsu.com>
Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor")
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/lib/libndctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ab47b27..226a577 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1649,6 +1649,7 @@ NDCTL_EXPORT unsigned int ndctl_dimm_get_health(struct ndctl_dimm *dimm)
 	}
 	if (ndctl_cmd_submit(cmd)) {
 		err(ctx, "%s: smart command failed\n", devname);
+		ndctl_cmd_unref(cmd);
 		return UINT_MAX;
 	}
 
@@ -1671,6 +1672,7 @@ NDCTL_EXPORT unsigned int ndctl_dimm_get_flags(struct ndctl_dimm *dimm)
 	}
 	if (ndctl_cmd_submit(cmd)) {
 		dbg(ctx, "%s: smart command failed\n", devname);
+		ndctl_cmd_unref(cmd);
 		return UINT_MAX;
 	}
 
@@ -1700,6 +1702,7 @@ NDCTL_EXPORT unsigned int ndctl_dimm_get_event_flags(struct ndctl_dimm *dimm)
 	}
 	if (ndctl_cmd_submit(cmd)) {
 		err(ctx, "%s: smart command failed\n", devname);
+		ndctl_cmd_unref(cmd);
 		return UINT_MAX;
 	}
 
-- 
2.14.4

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

[ndctl PATCH 4/5] ndctl, test: fix a potential null pointer dereference in 'ndctl test'

[Vishal Verma]

Static analysis reports that we can potentially dereference a null
pointer in the above function. Fix by checking the return value from the
allocation.

Cc: Dan Williams <dan.j.williams@intel.com>
Fixes: 8229da8d73a3 ("ndctl: ship the unit test as 'ndctl test'")
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ndctl/test.c b/ndctl/test.c
index 285594f..25ec61c 100644
--- a/ndctl/test.c
+++ b/ndctl/test.c
@@ -55,6 +55,8 @@ int cmd_test(int argc, const char **argv, void *ctx)
 		test = ndctl_test_new(UINT_MAX);
 	else
 		test = ndctl_test_new(0);
+	if (!test)
+		return EXIT_FAILURE;
 
 	rc = test_libndctl(loglevel, test, ctx);
 	fprintf(stderr, "test-libndctl: %s\n", result(rc));
-- 
2.14.4

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

[ndctl PATCH 5/5] ndctl, test: fix a resource leak in check_smart_threshold

[Vishal Verma]

Static analysis reports that we leak ndctl_cmd in the above function.
Fix by adding the cmd_unref to the exit path in the
__check_smart_threshold macro.

Cc: Dan Williams <dan.j.williams@intel.com>
Fixes: 7fa8a6af6917 ("test, libndctl: fix SMART test assumptions")
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 test/libndctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/libndctl.c b/test/libndctl.c
index edff0af..e36c1fb 100644
--- a/test/libndctl.c
+++ b/test/libndctl.c
@@ -2250,6 +2250,7 @@ static int check_smart(struct ndctl_bus *bus, struct ndctl_dimm *dimm,
 				ndctl_dimm_get_handle(dimm), \
 				smart_t_data.field, \
 				ndctl_cmd_smart_threshold_get_##field(cmd)); \
+		ndctl_cmd_unref(cmd_set); \
 		ndctl_cmd_unref(cmd); \
 		return -ENXIO; \
 	} \
-- 
2.14.4

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

Re: [ndctl PATCH 0/5] ndctl: misc static analysis fixes

[Dave Jiang]

On 08/10/2018 03:56 PM, Vishal Verma wrote:
> Fix up various static analysis reports from recent (and not-so-recent)
> patches.
> 
> Vishal Verma (5):
>   ndctl, inject: fix a resource leak in ndctl_namespace_get_clear_unit
>   ndctl: fix a resource leak in submit_get_firmware_info
>   libndctl: fix a resource leak in ndctl_dimm_get_{{event_}flags,
>     health}
>   ndctl, test: fix a potential null pointer dereference in 'ndctl test'
>   ndctl, test: fix a resource leak in check_smart_threshold
> 
>  ndctl/dimm.c         | 17 ++++++++++-------
>  ndctl/lib/inject.c   | 12 +++++++-----
>  ndctl/lib/libndctl.c |  3 +++
>  ndctl/test.c         |  2 ++
>  test/libndctl.c      |  1 +
>  5 files changed, 23 insertions(+), 12 deletions(-)
> 

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm