[PATCH 0/3] Misc. ndctl fixes

[PATCH 0/3] Misc. ndctl fixes

[Jeff Moyer]

Static checkers turned up a few potential problems in the code.  This
series fixes them.

Cheers,
Jeff

[PATCH 1/3] util_namespace_to_json: fix potential null pointer
[PATCH 2/3] daxctl_region_unref: fix a potential null pointer
[PATCH 3/3] validate_namespace_options: fix bogus test

[PATCH 1/3] util_namespace_to_json: fix potential null pointer dereference

[Jeff Moyer]

Coverity pointed out that "dax" could be NULL, and it goes unchecked.

Signed-off-by: Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 ndctl/util/json.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ndctl/util/json.c b/ndctl/util/json.c
index d144039..92e50fa 100644
--- a/ndctl/util/json.c
+++ b/ndctl/util/json.c
@@ -133,6 +133,8 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns)
 		jobj = json_object_new_string("memory");
 		break;
 	case NDCTL_NS_MODE_DAX:
+		if (!dax)
+			goto err;
 		size = ndctl_dax_get_size(dax);
 		jobj = json_object_new_string("dax");
 		break;
-- 
2.8.2.335.g4bb51ae

[PATCH 2/3] daxctl_region_unref: fix a potential null pointer dereference

[Jeff Moyer]

Move the assignment of ctx to after the NULL check.

Signed-off-by: Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 daxctl/lib/libdaxctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index a8ad58d..9d71a29 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -181,7 +181,7 @@ static void free_dev(struct daxctl_dev *dev, struct list_head *head)
 
 DAXCTL_EXPORT void daxctl_region_unref(struct daxctl_region *region)
 {
-	struct daxctl_ctx *ctx = region->ctx;
+	struct daxctl_ctx *ctx;
 	struct daxctl_dev *dev, *_d;
 
 	if (!region)
@@ -190,6 +190,7 @@ DAXCTL_EXPORT void daxctl_region_unref(struct daxctl_region *region)
 	if (region->refcount)
 		return;
 
+	ctx = region->ctx;
 	info(ctx, "region%d released\n", region->id);
 	list_for_each_safe(&region->devices, dev, _d, list)
 		free_dev(dev, &region->devices);
-- 
2.8.2.335.g4bb51ae

[PATCH 3/3] validate_namespace_options: fix bogus test

[Jeff Moyer]

Another coverity find.  The patch is self-explanatory, I hope.

Signed-off-by: Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 ndctl/builtin-xaction-namespace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ndctl/builtin-xaction-namespace.c b/ndctl/builtin-xaction-namespace.c
index 8ce7c3e..7411a01 100644
--- a/ndctl/builtin-xaction-namespace.c
+++ b/ndctl/builtin-xaction-namespace.c
@@ -444,7 +444,8 @@ static int validate_namespace_options(struct ndctl_region *region,
 				ndctl_namespace_get_devname(ndns));
 			return -EINVAL;
 		}
-	} else if (p->mode == NDCTL_NS_MODE_MEMORY || NDCTL_NS_MODE_DAX)
+	} else if (p->mode == NDCTL_NS_MODE_MEMORY ||
+		   p->mode == NDCTL_NS_MODE_DAX)
 		p->loc = NDCTL_PFN_LOC_PMEM;
 
 	/* check if we need, and whether the kernel supports, pfn devices */
-- 
2.8.2.335.g4bb51ae

Re: [PATCH 1/3] util_namespace_to_json: fix potential null pointer dereference

[Dan Williams]

On Thu, Jun 23, 2016 at 12:59 PM, Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Coverity pointed out that "dax" could be NULL, and it goes unchecked.
>
> Signed-off-by: Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  ndctl/util/json.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/ndctl/util/json.c b/ndctl/util/json.c
> index d144039..92e50fa 100644
> --- a/ndctl/util/json.c
> +++ b/ndctl/util/json.c
> @@ -133,6 +133,8 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns)
>                 jobj = json_object_new_string("memory");
>                 break;
>         case NDCTL_NS_MODE_DAX:
> +               if (!dax)
> +                       goto err;
>                 size = ndctl_dax_get_size(dax);
>                 jobj = json_object_new_string("dax");
>                 break;

The other patches look good, this one points out a problem with
ndctl_namespace_get_mode() which should be using a cached value rather
reading it from sysfs directly.  When we snapshot it with a cached
value it becomes impossible to have ndctl_namespace_get_mode() ==
NDCTL_NS_MODE_DAX and ndctl_namespace_get_dax() == NULL.