[ndctl PATCH 0/3] cxl: static analysis fixes

[ndctl PATCH 0/3] cxl: static analysis fixes

[Vishal Verma]

Fix a small handful of issues reported by scan.coverity.com for the
recent region management additions.

Vishal Verma (3):
  cxl/region: fix a dereferecnce after NULL check
  libcxl: fox a resource leak and a forward NULL check
  cxl/filter: Fix an uninitialized pointer dereference

 cxl/lib/libcxl.c | 3 ++-
 cxl/filter.c     | 2 +-
 cxl/region.c     | 2 --
 3 files changed, 3 insertions(+), 4 deletions(-)


base-commit: 9a993ce24fdd5de45774b65211570dd514cdf61d
-- 
2.37.2

[ndctl PATCH 1/3] cxl/region: fix a dereferecnce after NULL check

[Vishal Verma]

A NULL check in region_action() implies that 'decoder' might be NULL, but
later we dereference it during cxl_decoder_foreach().

Since cxl_decoder_foreach() won't ever enter the loop with a NULL decoder,
the check was superfluous. Remove it.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 cxl/region.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index a30313c..9372d6b 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -688,8 +688,6 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx,
 		cxl_decoder_foreach (port, decoder) {
 			decoder = util_cxl_decoder_filter(decoder,
 							  param.root_decoder);
-			if (!decoder)
-				continue;
 			rc = decoder_region_action(p, decoder, action, count);
 			if (rc)
 				err_rc = rc;
-- 
2.37.2

[ndctl PATCH 2/3] libcxl: fox a resource leak and a forward NULL check

[Vishal Verma]

Static analysis reports a couple of issues in add_cxl_region(). Firstly,
'path' wasn't freed in the success case, only in the error case.
Secondly, the error handling after 'calloc()'ing the region object
erroneously jumped to the error path which tried to free the region object,
instead of directly returning NULL.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 cxl/lib/libcxl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 021d59f..9945fd1 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -482,7 +482,7 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
 
 	region = calloc(1, sizeof(*region));
 	if (!region)
-		goto err;
+		return NULL;
 
 	region->id = id;
 	region->ctx = ctx;
@@ -551,6 +551,7 @@ static void *add_cxl_region(void *parent, int id, const char *cxlregion_base)
 
 	list_add_sorted(&decoder->regions, region, list, region_start_cmp);
 
+	free(path);
 	return region;
 err:
 	free(region->dev_path);
-- 
2.37.2

[ndctl PATCH 3/3] cxl/filter: Fix an uninitialized pointer dereference

[Vishal Verma]

Static analysis points out that there was a chance that 'jdecoder' could
be used while uninitialized in walk_decoders(). Initialize it to NULL to
avoid this.

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 cxl/filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cxl/filter.c b/cxl/filter.c
index 9a3de8c..56c6599 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -796,7 +796,7 @@ static void walk_decoders(struct cxl_port *port, struct cxl_filter_params *p,
 	cxl_decoder_foreach(port, decoder) {
 		const char *devname = cxl_decoder_get_devname(decoder);
 		struct json_object *jchildregions = NULL;
-		struct json_object *jdecoder;
+		struct json_object *jdecoder = NULL;
 
 		if (!p->decoders)
 			goto walk_children;
-- 
2.37.2

Re: [ndctl PATCH 1/3] cxl/region: fix a dereferecnce after NULL check

[Verma, Vishal L]

On Tue, 2022-08-23 at 01:21 -0600, Vishal Verma wrote:
> A NULL check in region_action() implies that 'decoder' might be NULL, but
> later we dereference it during cxl_decoder_foreach().
> 
> Since cxl_decoder_foreach() won't ever enter the loop with a NULL decoder,
> the check was superfluous. Remove it.
> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  cxl/region.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/cxl/region.c b/cxl/region.c
> index a30313c..9372d6b 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -688,8 +688,6 @@ static int region_action(int argc, const char **argv, struct cxl_ctx *ctx,
>                 cxl_decoder_foreach (port, decoder) {
>                         decoder = util_cxl_decoder_filter(decoder,
>                                                           param.root_decoder);
> -                       if (!decoder)
> -                               continue;

Hm, this is actually wrong. We need to save the filter results in a new
variable, and NULL check that, while keeping the original 'decoder'
variable intact for the loop. I'll send v2.

>                         rc = decoder_region_action(p, decoder, action, count);
>                         if (rc)
>                                 err_rc = rc;