All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/39] multipath: coverity fixes
@ 2016-06-16  9:47 Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 01/39] multipathd: correctly free refwwid in cli_add_map() Hannes Reinecke
                   ` (39 more replies)
  0 siblings, 40 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: dm-devel

Hi all,

here's a bunch of fixes from a recent coverity run.
Nothing critical, mostly memory leaks and possible
buffer overflows. And the usual missing return value
check.

As usual, comments and reviews are welcome.

The whole patchset can be found at
github.com:/hreinecke/multipath-tools.git
branch coverity

Hannes Reinecke (39):
  multipathd: correctly free refwwid in cli_add_map()
  libmultipath: fixup string copy and comparison
  libmultipath: fixup uninitialized return value in dm_reassign_table()
  libmultipath: free pathgroup in group_by_prio()
  libmultipath: memory leak in remove_feature()
  libmultipath: memory leak in add_map_without_path()
  libmultipath: missing NULL check in set_pgfailback()
  libmultipath: remove stale references to fd_ep in uevent_listen()
  libmultipath/devmapper.c: Remove unused 'next' argument
  libmultipath/alias.c: check return value from strchr()
  libmultipath/devmapper.c: check for errors from sscanf()
  libmultipath/uevent.c: handle errors from setsockopt()
  libmultipath/discovery.c: handle errors from sscanf()
  libmultipath: drop unused variable 'diop' in libsg.c
  libmultipath: use fstat() when reading sysfs attributes
  libmultipath: fixup possible buffer overflow in alua_rtpg.c
  libmultipath: memory leak in iet.c
  libmpathcmd: limit size of the CLI buffer
  libmpathpersist: Fix possible overrun in devt2devname()
  libmpathpersist: memory leak in mpath_persistent_reserve_(in,out)()
  libmpathpersist: Pass sensebuffer as pointer
  libmpathpersist: Handle send error
  libmultipath: missing break in hds prioritizer
  kpartx: fixup fd leakage in lopart.c
  kpartx: Memory leak in get_hotplug_device()
  kpartx: fixup filedescriptor leak in set_loop()
  kpartx: do not use 'const' for mapname in get_hotplug_device()
  kpartx: Handle errors from lseek()
  kpartx/devmapper.c: Remove unused 'next' argument
  kpartx: use fstat() when reading sysfs attributes
  kpartx/lopart.c: use fstat() when reading device nodes
  libmultipath/sysfs.c: always terminate value from
    sysfs_attr_get_value()
  multipathd: missing mpath_disconnect() in uxclnt()
  multipathd: pid fd resource leak
  libmultipath: mark return value from dup() as unused
  libmultipath: Ignore errors from fcntl() in directio.c
  libmultipath: add spin_lock in tur.c
  libmultipath/devmapper.c: fixup possible buffer overflow
  libmultipath/discovery.c: Fixup possible range overflow

 kpartx/devmapper.c                    |  5 ++-
 kpartx/gpt.c                          |  5 +--
 kpartx/kpartx.c                       | 13 ++++---
 kpartx/lopart.c                       | 39 ++++++++++-----------
 libmpathcmd/mpath_cmd.c               |  2 ++
 libmpathcmd/mpath_cmd.h               |  1 +
 libmpathpersist/mpath_persist.c       | 21 +++++++++---
 libmpathpersist/mpath_pr_ioctl.c      | 35 ++++++++++---------
 libmpathpersist/mpath_updatepr.c      |  8 +++--
 libmultipath/alias.c                  |  3 +-
 libmultipath/callout.c                |  4 ++-
 libmultipath/checkers/directio.c      |  4 ++-
 libmultipath/checkers/libsg.c         |  3 --
 libmultipath/checkers/tur.c           |  4 ++-
 libmultipath/configure.c              | 10 +++---
 libmultipath/devmapper.c              | 40 +++++++++++++---------
 libmultipath/dict.c                   |  2 ++
 libmultipath/discovery.c              | 13 +++----
 libmultipath/dmparser.c               | 16 +++++----
 libmultipath/pgpolicies.c             | 14 +++++---
 libmultipath/prio.c                   |  2 +-
 libmultipath/prioritizers/alua_rtpg.c |  8 ++++-
 libmultipath/prioritizers/hds.c       |  2 ++
 libmultipath/prioritizers/iet.c       |  3 +-
 libmultipath/structs.c                |  4 ++-
 libmultipath/structs_vec.c            | 10 ++++--
 libmultipath/sysfs.c                  | 64 ++++++++++++++++++++---------------
 libmultipath/uevent.c                 | 19 +++++++----
 libmultipath/waiter.c                 |  2 +-
 multipathd/cli_handlers.c             |  2 +-
 multipathd/main.c                     |  6 +++-
 multipathd/pidfile.c                  |  6 ++--
 multipathd/uxclnt.c                   |  2 +-
 33 files changed, 226 insertions(+), 146 deletions(-)

-- 
2.6.6

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 01/39] multipathd: correctly free refwwid in cli_add_map()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 02/39] libmultipath: fixup string copy and comparison Hannes Reinecke
                   ` (38 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We need to free the refwwid only when present.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 multipathd/cli_handlers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index a1b7052..52bcdaa 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -715,6 +715,7 @@ cli_add_map (void * v, char ** reply, int * len, void * data)
 					condlog(2, "%s: coalesce_paths failed",
 									param);
 				dm_lib_release();
+				FREE(refwwid);
 			}
 		} /*we attempt to create device only once*/
 		count++;
@@ -726,7 +727,6 @@ cli_add_map (void * v, char ** reply, int * len, void * data)
 	}
 	rc = ev_add_map(dev_path, alias, vecs);
 	FREE(alias);
-	FREE(refwwid);
 	return rc;
 }
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 02/39] libmultipath: fixup string copy and comparison
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 01/39] multipathd: correctly free refwwid in cli_add_map() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 03/39] libmultipath: fixup uninitialized return value in dm_reassign_table() Hannes Reinecke
                   ` (37 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

When doing a strncpy or strncmp we need to omit the trailing
NULL in the length to avoid any possible overflow.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmpathpersist/mpath_persist.c | 11 +++++++----
 libmultipath/configure.c        | 10 +++++-----
 libmultipath/dmparser.c         | 16 ++++++++++------
 libmultipath/prio.c             |  2 +-
 libmultipath/structs_vec.c      |  4 ++--
 libmultipath/waiter.c           |  2 +-
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index b23e116..d2c3e53 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -375,7 +375,8 @@ get_mpvec (vector curmp, vector pathvec, char * refwwid)
 		/*
 		 * discard out of scope maps
 		 */
-		if (mpp->alias && refwwid && strncmp (mpp->alias, refwwid, WWID_SIZE)){
+		if (mpp->alias && refwwid &&
+		    strncmp (mpp->alias, refwwid, WWID_SIZE - 1)){
 			free_multipath (mpp, KEEP_PATHS);
 			vector_del_slot (curmp, i);
 			i--;
@@ -485,7 +486,8 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 				condlog (1, "%s: %s path not up. Skip.", mpp->wwid, pp->dev);
 				continue;
 			}
-			strncpy(thread[count].param.dev, pp->dev, FILE_NAME_SIZE);
+			strncpy(thread[count].param.dev, pp->dev,
+				FILE_NAME_SIZE - 1);
 
 			if (count && (thread[count].param.paramp->sa_flags & MPATH_F_SPEC_I_PT_MASK)){
 				/*
@@ -602,7 +604,7 @@ int send_prout_activepath(char * dev, int rq_servact, int rq_scope,
 	int rc;
 
 	memset(&thread, 0, sizeof(thread));
-	strncpy(param.dev, dev, FILE_NAME_SIZE);
+	strncpy(param.dev, dev, FILE_NAME_SIZE - 1);
 	/* Initialize and set thread joinable attribute */
 	pthread_attr_init(&attr);
 	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);
@@ -670,7 +672,8 @@ int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope,
 				continue;
 			}
 
-			strncpy(thread[count].param.dev, pp->dev, FILE_NAME_SIZE);
+			strncpy(thread[count].param.dev, pp->dev,
+				FILE_NAME_SIZE - 1);
 			condlog (3, "%s: sending pr out command to %s", mpp->wwid, pp->dev);
 			rc = pthread_create (&thread[count].id, &attr, mpath_prout_pthread_fn,
 					(void *) (&thread[count].param));
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index a4a2c44..8e938c0 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -65,7 +65,7 @@ int group_by_host_adapter(struct pathgroup *pgp, vector adapters)
 			goto out;
 		agp->pgp = pgp;
 
-		strncpy(agp->adapter_name, adapter_name1, SLOT_NAME_SIZE);
+		strncpy(agp->adapter_name, adapter_name1, SLOT_NAME_SIZE - 1);
 		store_adaptergroup(adapters, agp);
 
 		/* create a new host port group
@@ -395,7 +395,7 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
 		if (cmpp) {
 			condlog(2, "%s: rename %s to %s", mpp->wwid,
 				cmpp->alias, mpp->alias);
-			strncpy(mpp->alias_old, cmpp->alias, WWID_SIZE);
+			strncpy(mpp->alias_old, cmpp->alias, WWID_SIZE - 1);
 			mpp->action = ACT_RENAME;
 			if (force_reload)
 				mpp->action = ACT_FORCERENAME;
@@ -410,7 +410,7 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
 	if (!cmpp) {
 		condlog(2, "%s: remove (wwid changed)", mpp->alias);
 		dm_flush_map(mpp->alias);
-		strncpy(cmpp_by_name->wwid, mpp->wwid, WWID_SIZE);
+		strncpy(cmpp_by_name->wwid, mpp->wwid, WWID_SIZE - 1);
 		drop_multipath(curmp, cmpp_by_name->wwid, KEEP_PATHS);
 		mpp->action = ACT_CREATE;
 		condlog(3, "%s: set ACT_CREATE (map wwid change)",
@@ -770,7 +770,7 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 		}
 
 		/* 4. path is out of scope */
-		if (refwwid && strncmp(pp1->wwid, refwwid, WWID_SIZE))
+		if (refwwid && strncmp(pp1->wwid, refwwid, WWID_SIZE - 1))
 			continue;
 
 		/* If find_multipaths was selected check if the path is valid */
@@ -896,7 +896,7 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 			if (!deadmap(mpp))
 				continue;
 
-			strncpy(alias, mpp->alias, WWID_SIZE);
+			strncpy(alias, mpp->alias, WWID_SIZE - 1);
 
 			if ((j = find_slot(newmp, (void *)mpp)) != -1)
 				vector_del_slot(newmp, j);
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 289be89..98fb559 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -344,10 +344,11 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
 				if (!pp)
 					goto out1;
 
-				strncpy(pp->dev_t, word, BLK_DEV_SIZE);
-				strncpy(pp->dev, devname, FILE_NAME_SIZE);
+				strncpy(pp->dev_t, word, BLK_DEV_SIZE - 1);
+				strncpy(pp->dev, devname, FILE_NAME_SIZE - 1);
 				if (strlen(mpp->wwid)) {
-					strncpy(pp->wwid, mpp->wwid, WWID_SIZE);
+					strncpy(pp->wwid, mpp->wwid,
+						WWID_SIZE - 1);
 				}
 				/* Only call this in multipath client mode */
 				if (!conf->daemon && store_path(pathvec, pp))
@@ -355,7 +356,8 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
 			} else {
 				if (!strlen(pp->wwid) &&
 				    strlen(mpp->wwid))
-					strncpy(pp->wwid, mpp->wwid, WWID_SIZE);
+					strncpy(pp->wwid, mpp->wwid,
+						WWID_SIZE - 1);
 			}
 			FREE(word);
 
@@ -367,14 +369,16 @@ disassemble_map (vector pathvec, char * params, struct multipath * mpp)
 			 * in the get_dm_mpvec() code path
 			 */
 			if (!strlen(mpp->wwid))
-				strncpy(mpp->wwid, pp->wwid, WWID_SIZE);
+				strncpy(mpp->wwid, pp->wwid,
+					WWID_SIZE - 1);
 
 			/*
 			 * Update wwid for paths which may not have been
 			 * active at the time the getuid callout was run
 			 */
 			else if (!strlen(pp->wwid))
-				strncpy(pp->wwid, mpp->wwid, WWID_SIZE);
+				strncpy(pp->wwid, mpp->wwid,
+					WWID_SIZE - 1);
 
 			pgp->id ^= (long)pp;
 			pp->pgindex = i + 1;
diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index ab8eca9..fbf3190 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -158,7 +158,7 @@ void prio_get (struct prio * dst, char * name, char * args)
 
 	strncpy(dst->name, src->name, PRIO_NAME_LEN);
 	if (args)
-		strncpy(dst->args, args, PRIO_ARGS_LEN);
+		strncpy(dst->args, args, PRIO_ARGS_LEN - 1);
 	dst->getprio = src->getprio;
 	dst->handle = NULL;
 
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 7d46d42..e992b54 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -447,8 +447,8 @@ find_existing_alias (struct multipath * mpp,
 	int i;
 
 	vector_foreach_slot (vecs->mpvec, mp, i)
-		if (strcmp(mp->wwid, mpp->wwid) == 0) {
-			strncpy(mpp->alias_old, mp->alias, WWID_SIZE);
+		if (strncmp(mp->wwid, mpp->wwid, WWID_SIZE - 1) == 0) {
+			strncpy(mpp->alias_old, mp->alias, WWID_SIZE - 1);
 			return;
 		}
 }
diff --git a/libmultipath/waiter.c b/libmultipath/waiter.c
index 7cedd4b..6937034 100644
--- a/libmultipath/waiter.c
+++ b/libmultipath/waiter.c
@@ -191,7 +191,7 @@ int start_waiter_thread (struct multipath *mpp, struct vectors *vecs)
 	if (!wp)
 		goto out;
 
-	strncpy(wp->mapname, mpp->alias, WWID_SIZE);
+	strncpy(wp->mapname, mpp->alias, WWID_SIZE - 1);
 	wp->vecs = vecs;
 
 	if (pthread_create(&wp->thread, &waiter_attr, waitevent, wp)) {
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 03/39] libmultipath: fixup uninitialized return value in dm_reassign_table()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 01/39] multipathd: correctly free refwwid in cli_add_map() Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 02/39] libmultipath: fixup string copy and comparison Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 04/39] libmultipath: free pathgroup in group_by_prio() Hannes Reinecke
                   ` (36 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/devmapper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index ce884e2..6e675fe 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1447,7 +1447,7 @@ void dm_reassign_deps(char *table, char *dep, char *newdep)
 
 int dm_reassign_table(const char *name, char *old, char *new)
 {
-	int r, modified = 0;
+	int r = 0, modified = 0;
 	uint64_t start, length;
 	struct dm_task *dmt, *reload_dmt;
 	char *target, *params = NULL;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 04/39] libmultipath: free pathgroup in group_by_prio()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (2 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 03/39] libmultipath: fixup uninitialized return value in dm_reassign_table() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 05/39] libmultipath: memory leak in remove_feature() Hannes Reinecke
                   ` (35 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

If we encounter an error when storing the pathgroup or paths
the pathgroup is never deallocated.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/pgpolicies.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libmultipath/pgpolicies.c b/libmultipath/pgpolicies.c
index 2981d51..f440441 100644
--- a/libmultipath/pgpolicies.c
+++ b/libmultipath/pgpolicies.c
@@ -341,8 +341,10 @@ group_by_prio (struct multipath * mp)
 		if (!pgp)
 			goto out;
 
-		if (store_path(pgp->paths, VECTOR_SLOT(mp->paths, 0)))
-				goto out;
+		if (store_path(pgp->paths, VECTOR_SLOT(mp->paths, 0))) {
+			free_pathgroup(pgp, KEEP_PATHS);
+			goto out;
+		}
 
 		vector_del_slot(mp->paths, 0);
 
@@ -350,11 +352,15 @@ group_by_prio (struct multipath * mp)
 		 * Store the new path group into the vector.
 		 */
 		if (i < VECTOR_SIZE(mp->pg)) {
-			if (!vector_insert_slot(mp->pg, i, pgp))
+			if (!vector_insert_slot(mp->pg, i, pgp)) {
+				free_pathgroup(pgp, KEEP_PATHS);
 				goto out;
+			}
 		} else {
-			if (store_pathgroup(mp->pg, pgp))
+			if (store_pathgroup(mp->pg, pgp)) {
+				free_pathgroup(pgp, KEEP_PATHS);
 				goto out;
+			}
 		}
 
 		/*
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 05/39] libmultipath: memory leak in remove_feature()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (3 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 04/39] libmultipath: free pathgroup in group_by_prio() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 06/39] libmultipath: memory leak in add_map_without_path() Hannes Reinecke
                   ` (34 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Upon error the 'n' string was never freed.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/structs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 502d98e..ed62b07 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -653,9 +653,11 @@ remove_feature(char **f, char *o)
 	 * about to be removed
 	 */
 	p = strchr(*f, ' ');
-	if (!p)
+	if (!p) {
 		/* Internal error, feature string inconsistent */
+		FREE(n);
 		return 1;
+	}
 	while (*p == ' ')
 		p++;
 	p--;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 06/39] libmultipath: memory leak in add_map_without_path()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (4 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 05/39] libmultipath: memory leak in remove_feature() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 07/39] libmultipath: missing NULL check in set_pgfailback() Hannes Reinecke
                   ` (33 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

If we call add_map_without_path() with an empty alias the
mpp structure is never freed.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/structs_vec.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index e992b54..20a7457 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -414,8 +414,12 @@ add_map_without_path (struct vectors * vecs, char * alias)
 {
 	struct multipath * mpp = alloc_multipath();
 
-	if (!mpp || !alias)
+	if (!mpp)
+		return NULL;
+	if (!alias) {
+		FREE(mpp);
 		return NULL;
+	}
 
 	mpp->alias = STRDUP(alias);
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 07/39] libmultipath: missing NULL check in set_pgfailback()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (5 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 06/39] libmultipath: memory leak in add_map_without_path() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 08/39] libmultipath: remove stale references to fd_ep in uevent_listen() Hannes Reinecke
                   ` (32 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/dict.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 7f912ec..9920a9b 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -805,6 +805,8 @@ set_pgfailback(vector strvec, void *ptr)
 	char * buff;
 
 	buff = set_value(strvec);
+	if (!buff)
+		return 1;
 
 	if (strlen(buff) == 6 && !strcmp(buff, "manual"))
 		*int_ptr = -FAILBACK_MANUAL;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 08/39] libmultipath: remove stale references to fd_ep in uevent_listen()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (6 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 07/39] libmultipath: missing NULL check in set_pgfailback() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 09/39] libmultipath/devmapper.c: Remove unused 'next' argument Hannes Reinecke
                   ` (31 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/uevent.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 454b819..fb8b4a7 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -474,7 +474,7 @@ int uevent_listen(struct udev *udev)
 {
 	int err = 2;
 	struct udev_monitor *monitor = NULL;
-	int fd, fd_ep = -1, socket_flags, events;
+	int fd, socket_flags, events;
 	int need_failback = 1;
 	int timeout = 30;
 	LIST_HEAD(uevlisten_tmp);
@@ -579,8 +579,6 @@ int uevent_listen(struct udev *udev)
 	}
 	need_failback = 0;
 out:
-	if (fd_ep >= 0)
-		close(fd_ep);
 	if (monitor)
 		udev_monitor_unref(monitor);
 	if (need_failback)
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 09/39] libmultipath/devmapper.c: Remove unused 'next' argument
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (7 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 08/39] libmultipath: remove stale references to fd_ep in uevent_listen() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 10/39] libmultipath/alias.c: check return value from strchr() Hannes Reinecke
                   ` (30 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/devmapper.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 6e675fe..f395481 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -421,7 +421,6 @@ dm_get_map(const char * name, unsigned long long * size, char * outparams)
 {
 	int r = 1;
 	struct dm_task *dmt;
-	void *next = NULL;
 	uint64_t start, length;
 	char *target_type = NULL;
 	char *params = NULL;
@@ -438,8 +437,8 @@ dm_get_map(const char * name, unsigned long long * size, char * outparams)
 		goto out;
 
 	/* Fetch 1st target */
-	next = dm_get_next_target(dmt, next, &start, &length,
-				  &target_type, &params);
+	dm_get_next_target(dmt, NULL, &start, &length,
+			   &target_type, &params);
 
 	if (size)
 		*size = length;
@@ -530,7 +529,6 @@ dm_get_status(char * name, char * outstatus)
 {
 	int r = 1;
 	struct dm_task *dmt;
-	void *next = NULL;
 	uint64_t start, length;
 	char *target_type;
 	char *status;
@@ -547,8 +545,8 @@ dm_get_status(char * name, char * outstatus)
 		goto out;
 
 	/* Fetch 1st target */
-	next = dm_get_next_target(dmt, next, &start, &length,
-				  &target_type, &status);
+	dm_get_next_target(dmt, NULL, &start, &length,
+			   &target_type, &status);
 
 	if (snprintf(outstatus, PARAMS_SIZE, "%s", status) <= PARAMS_SIZE)
 		r = 0;
@@ -571,7 +569,6 @@ dm_type(const char * name, char * type)
 {
 	int r = 0;
 	struct dm_task *dmt;
-	void *next = NULL;
 	uint64_t start, length;
 	char *target_type = NULL;
 	char *params;
@@ -588,8 +585,8 @@ dm_type(const char * name, char * type)
 		goto out;
 
 	/* Fetch 1st target */
-	next = dm_get_next_target(dmt, next, &start, &length,
-				  &target_type, &params);
+	dm_get_next_target(dmt, NULL, &start, &length,
+			   &target_type, &params);
 
 	if (!target_type)
 		r = -1;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 10/39] libmultipath/alias.c: check return value from strchr()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (8 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 09/39] libmultipath/devmapper.c: Remove unused 'next' argument Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 11/39] libmultipath/devmapper.c: check for errors from sscanf() Hannes Reinecke
                   ` (29 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/alias.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index 5fdcdb5..b86843a 100644
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
@@ -229,7 +229,8 @@ allocate_binding(int fd, char *wwid, int id, char *prefix)
 		return NULL;
 	}
 	c = strchr(buf, ' ');
-	*c = '\0';
+	if (c)
+		*c = '\0';
 	alias = strdup(buf);
 	if (alias == NULL)
 		condlog(0, "cannot copy new alias from bindings file : %s",
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 11/39] libmultipath/devmapper.c: check for errors from sscanf()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (9 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 10/39] libmultipath/alias.c: check return value from strchr() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 12/39] libmultipath/uevent.c: handle errors from setsockopt() Hannes Reinecke
                   ` (28 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/devmapper.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index f395481..0223e18 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -116,7 +116,10 @@ dm_lib_prereq (void)
 
 	dm_get_library_version(version, sizeof(version));
 	condlog(3, "libdevmapper version %s", version);
-	sscanf(version, "%d.%d.%d ", &v[0], &v[1], &v[2]);
+	if (sscanf(version, "%d.%d.%d ", &v[0], &v[1], &v[2]) != 3) {
+		condlog(0, "invalid libdevmapper version %s", version);
+		return 1;
+	}
 
 	if VERSION_GE(v, minv)
 		return 0;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 12/39] libmultipath/uevent.c: handle errors from setsockopt()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (10 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 11/39] libmultipath/devmapper.c: check for errors from sscanf() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 13/39] libmultipath/discovery.c: handle errors from sscanf() Hannes Reinecke
                   ` (27 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/uevent.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index fb8b4a7..ff4bc86 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -291,8 +291,12 @@ int failback_listen(void)
 		}
 
 		/* enable receiving of the sender credentials */
-		setsockopt(sock, SOL_SOCKET, SO_PASSCRED,
-			   &feature_on, sizeof(feature_on));
+		retval = setsockopt(sock, SOL_SOCKET, SO_PASSCRED,
+				    &feature_on, sizeof(feature_on));
+		if (retval < 0) {
+			condlog(0, "failed to enable credential passing, exit");
+			goto exit;
+		}
 
 	} else {
 		/* Fallback to read kernel netlink events */
@@ -330,8 +334,11 @@ int failback_listen(void)
 		condlog(3, "receive buffer size for socket is %u.", rcvsz);
 
 		/* enable receiving of the sender credentials */
-		setsockopt(sock, SOL_SOCKET, SO_PASSCRED,
-			   &feature_on, sizeof(feature_on));
+		if (setsockopt(sock, SOL_SOCKET, SO_PASSCRED,
+			       &feature_on, sizeof(feature_on)) < 0) {
+			condlog(0, "error on enabling credential passing for socket");
+			exit(1);
+		}
 
 		retval = bind(sock, (struct sockaddr *) &snl,
 			      sizeof(struct sockaddr_nl));
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 13/39] libmultipath/discovery.c: handle errors from sscanf()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (11 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 12/39] libmultipath/uevent.c: handle errors from setsockopt() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 14/39] libmultipath: drop unused variable 'diop' in libsg.c Hannes Reinecke
                   ` (26 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/discovery.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index db82b00..5d5441d 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1179,16 +1179,17 @@ ccw_sysfs_pathinfo (struct path * pp)
 	 */
 	attr_path = udev_device_get_sysname(parent);
 	pp->sg_id.lun = 0;
-	sscanf(attr_path, "%i.%i.%x",
-			&pp->sg_id.host_no,
-			&pp->sg_id.channel,
-			&pp->sg_id.scsi_id);
-	condlog(3, "%s: h:b:t:l = %i:%i:%i:%i",
+	if (sscanf(attr_path, "%i.%i.%x",
+		   &pp->sg_id.host_no,
+		   &pp->sg_id.channel,
+		   &pp->sg_id.scsi_id) == 3) {
+		condlog(3, "%s: h:b:t:l = %i:%i:%i:%i",
 			pp->dev,
 			pp->sg_id.host_no,
 			pp->sg_id.channel,
 			pp->sg_id.scsi_id,
 			pp->sg_id.lun);
+	}
 
 	return 0;
 }
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 14/39] libmultipath: drop unused variable 'diop' in libsg.c
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (12 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 13/39] libmultipath/discovery.c: handle errors from sscanf() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 15/39] libmultipath: use fstat() when reading sysfs attributes Hannes Reinecke
                   ` (25 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/checkers/libsg.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libmultipath/checkers/libsg.c b/libmultipath/checkers/libsg.c
index 0d3af1f..958ea92 100644
--- a/libmultipath/checkers/libsg.c
+++ b/libmultipath/checkers/libsg.c
@@ -19,7 +19,6 @@ sg_read (int sg_fd, unsigned char * buff, int buff_len,
 	long long start_block = 0;
 	int bs = 512;
 	int cdbsz = 10;
-	int * diop = NULL;
 
 	unsigned char rdCmd[cdbsz];
 	unsigned char *sbb = sense;
@@ -55,8 +54,6 @@ sg_read (int sg_fd, unsigned char * buff, int buff_len,
 	io_hdr.sbp = sense;
 	io_hdr.timeout = timeout * 1000;
 	io_hdr.pack_id = (int)start_block;
-	if (diop && *diop)
-	io_hdr.flags |= SG_FLAG_DIRECT_IO;
 
 retry:
 	memset(sense, 0, sense_len);
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 15/39] libmultipath: use fstat() when reading sysfs attributes
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (13 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 14/39] libmultipath: drop unused variable 'diop' in libsg.c Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 16/39] libmultipath: fixup possible buffer overflow in alua_rtpg.c Hannes Reinecke
                   ` (24 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Coverity pointed out that we shouldn't be using stat()/open(),
as the file might have vanished or changed in between those
two calls. So modify it to open()/fstat() instead.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/sysfs.c | 59 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index de7df40..a74de9f 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -57,30 +57,31 @@ ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
 	snprintf(devpath, PATH_SIZE, "%s/%s", udev_device_get_syspath(dev),
 		 attr_name);
 	condlog(4, "open '%s'", devpath);
-	if (stat(devpath, &statbuf) != 0) {
+	/* read attribute value */
+	fd = open(devpath, O_RDONLY);
+	if (fd < 0) {
+		condlog(4, "attribute '%s' can not be opened: %s",
+			devpath, strerror(errno));
+		return -errno;
+	}
+	if (fstat(fd, &statbuf) < 0) {
 		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
+		close(fd);
 		return -ENXIO;
 	}
-
 	/* skip directories */
 	if (S_ISDIR(statbuf.st_mode)) {
 		condlog(4, "%s is a directory", devpath);
+		close(fd);
 		return -EISDIR;
 	}
-
 	/* skip non-writeable files */
 	if ((statbuf.st_mode & S_IRUSR) == 0) {
 		condlog(4, "%s is not readable", devpath);
+		close(fd);
 		return -EPERM;
 	}
 
-	/* read attribute value */
-	fd = open(devpath, O_RDONLY);
-	if (fd < 0) {
-		condlog(4, "attribute '%s' can not be opened: %s",
-			devpath, strerror(errno));
-		return -errno;
-	}
 	size = read(fd, value, value_len);
 	if (size < 0) {
 		condlog(4, "read from %s failed: %s", devpath, strerror(errno));
@@ -112,30 +113,33 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
 	snprintf(devpath, PATH_SIZE, "%s/%s", udev_device_get_syspath(dev),
 		 attr_name);
 	condlog(4, "open '%s'", devpath);
-	if (stat(devpath, &statbuf) != 0) {
+	/* read attribute value */
+	fd = open(devpath, O_RDONLY);
+	if (fd < 0) {
+		condlog(4, "attribute '%s' can not be opened: %s",
+			devpath, strerror(errno));
+		return -errno;
+	}
+	if (fstat(fd, &statbuf) != 0) {
 		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
+		close(fd);
 		return -ENXIO;
 	}
 
 	/* skip directories */
 	if (S_ISDIR(statbuf.st_mode)) {
 		condlog(4, "%s is a directory", devpath);
+		close(fd);
 		return -EISDIR;
 	}
 
 	/* skip non-writeable files */
 	if ((statbuf.st_mode & S_IRUSR) == 0) {
 		condlog(4, "%s is not readable", devpath);
+		close(fd);
 		return -EPERM;
 	}
 
-	/* read attribute value */
-	fd = open(devpath, O_RDONLY);
-	if (fd < 0) {
-		condlog(4, "attribute '%s' can not be opened: %s",
-			devpath, strerror(errno));
-		return -errno;
-	}
 	size = read(fd, value, value_len);
 	if (size < 0) {
 		condlog(4, "read from %s failed: %s", devpath, strerror(errno));
@@ -163,30 +167,33 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 	snprintf(devpath, PATH_SIZE, "%s/%s", udev_device_get_syspath(dev),
 		 attr_name);
 	condlog(4, "open '%s'", devpath);
-	if (stat(devpath, &statbuf) != 0) {
+	/* write attribute value */
+	fd = open(devpath, O_WRONLY);
+	if (fd < 0) {
+		condlog(4, "attribute '%s' can not be opened: %s",
+			devpath, strerror(errno));
+		return -errno;
+	}
+	if (fstat(fd, &statbuf) != 0) {
 		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
+		close(fd);
 		return -errno;
 	}
 
 	/* skip directories */
 	if (S_ISDIR(statbuf.st_mode)) {
 		condlog(4, "%s is a directory", devpath);
+		close(fd);
 		return -EISDIR;
 	}
 
 	/* skip non-writeable files */
 	if ((statbuf.st_mode & S_IWUSR) == 0) {
 		condlog(4, "%s is not writeable", devpath);
+		close(fd);
 		return -EPERM;
 	}
 
-	/* write attribute value */
-	fd = open(devpath, O_WRONLY);
-	if (fd < 0) {
-		condlog(4, "attribute '%s' can not be opened: %s",
-			devpath, strerror(errno));
-		return -errno;
-	}
 	size = write(fd, value, value_len);
 	if (size < 0) {
 		condlog(4, "write to %s failed: %s", devpath, strerror(errno));
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 16/39] libmultipath: fixup possible buffer overflow in alua_rtpg.c
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (14 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 15/39] libmultipath: use fstat() when reading sysfs attributes Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 17/39] libmultipath: memory leak in iet.c Hannes Reinecke
                   ` (23 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We need to reserve an additional 4 bytes for the length of
the response buffer, so add a proper range check to avoid
accidental wrap-arounds.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/prioritizers/alua_rtpg.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
index 636aae5..22b0d4f 100644
--- a/libmultipath/prioritizers/alua_rtpg.c
+++ b/libmultipath/prioritizers/alua_rtpg.c
@@ -15,6 +15,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 #include <inttypes.h>
 #include <libudev.h>
@@ -219,6 +220,9 @@ get_target_port_group(struct path * pp)
 			goto out;
 
 		scsi_buflen = (buf[2] << 8 | buf[3]) + 4;
+		/* Paranoia */
+		if (scsi_buflen >= USHRT_MAX)
+			scsi_buflen = USHRT_MAX;
 		if (buflen < scsi_buflen) {
 			free(buf);
 			buf = (unsigned char *)malloc(scsi_buflen);
@@ -303,7 +307,7 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
 	struct rtpg_tpg_dscr *	dscr;
 	int			rc;
 	int			buflen;
-	uint32_t		scsi_buflen;
+	uint64_t		scsi_buflen;
 
 	buflen = 4096;
 	buf = (unsigned char *)malloc(buflen);
@@ -317,6 +321,8 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
 	if (rc < 0)
 		goto out;
 	scsi_buflen = (buf[0] << 24 | buf[1] << 16 | buf[2] << 8 | buf[3]) + 4;
+	if (scsi_buflen > UINT_MAX)
+		scsi_buflen = UINT_MAX;
 	if (buflen < scsi_buflen) {
 		free(buf);
 		buf = (unsigned char *)malloc(scsi_buflen);
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 17/39] libmultipath: memory leak in iet.c
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (15 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 16/39] libmultipath: fixup possible buffer overflow in alua_rtpg.c Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 18/39] libmpathcmd: limit size of the CLI buffer Hannes Reinecke
                   ` (22 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

'pmatch' was never freed.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/prioritizers/iet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libmultipath/prioritizers/iet.c b/libmultipath/prioritizers/iet.c
index 0bcc48b..3549665 100644
--- a/libmultipath/prioritizers/iet.c
+++ b/libmultipath/prioritizers/iet.c
@@ -59,10 +59,11 @@ char *find_regex(char * string, char * regex)
 				if (result) {
 					strncpy(result, &string[start], size);
 					result[size] = '\0';
+					free(pmatch);
 					return result;
 				}
 			}
-			else return NULL;
+			free(pmatch);
 		}
 	}
 	return NULL;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 18/39] libmpathcmd: limit size of the CLI buffer
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (16 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 17/39] libmultipath: memory leak in iet.c Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 19/39] libmpathpersist: Fix possible overrun in devt2devname() Hannes Reinecke
                   ` (21 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

A malicious program might inject a large buffer via the CLI and
causing the daemon to abort due to OOM.
So limit the maximum CLI buffer size to 64k.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmpathcmd/mpath_cmd.c | 2 ++
 libmpathcmd/mpath_cmd.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index 1aaf5ea..2290ecb 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -142,6 +142,8 @@ int mpath_recv_reply(int fd, char **reply, unsigned int timeout)
 	len = mpath_recv_reply_len(fd, timeout);
 	if (len <= 0)
 		return len;
+	if (len > MAX_REPLY_LEN)
+		return -EINVAL;
 	*reply = malloc(len);
 	if (!*reply)
 		return -1;
diff --git a/libmpathcmd/mpath_cmd.h b/libmpathcmd/mpath_cmd.h
index 4ec0291..f33f000 100644
--- a/libmpathcmd/mpath_cmd.h
+++ b/libmpathcmd/mpath_cmd.h
@@ -28,6 +28,7 @@ extern "C" {
 
 #define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
 #define DEFAULT_REPLY_TIMEOUT	1000
+#define MAX_REPLY_LEN		65536
 
 
 /*
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 19/39] libmpathpersist: Fix possible overrun in devt2devname()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (17 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 18/39] libmpathcmd: limit size of the CLI buffer Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 20/39] libmpathpersist: memory leak in mpath_persistent_reserve_(in, out)() Hannes Reinecke
                   ` (20 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

pp->dev is of size FILE_NAME_SIZE, not PATH_SIZE.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmpathpersist/mpath_persist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index d2c3e53..c4b8521 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -84,7 +84,7 @@ updatepaths (struct multipath * mpp)
 
 		vector_foreach_slot (pgp->paths, pp, j){
 			if (!strlen(pp->dev)){
-				if (devt2devname(pp->dev, PATH_SIZE,
+				if (devt2devname(pp->dev, FILE_NAME_SIZE,
 						 pp->dev_t)){
 					/*
 					 * path is not in sysfs anymore
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 20/39] libmpathpersist: memory leak in mpath_persistent_reserve_(in, out)()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (18 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 19/39] libmpathpersist: Fix possible overrun in devt2devname() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 21/39] libmpathpersist: Pass sensebuffer as pointer Hannes Reinecke
                   ` (19 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

When either curmp or pathvec couldn't be allocated we never
free up the other one, too.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmpathpersist/mpath_persist.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index c4b8521..cfc2f0f 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -195,6 +195,10 @@ int mpath_persistent_reserve_in (int fd, int rq_servact,
 	if (!curmp || !pathvec){
 		condlog (0, "%s: vector allocation failed.", alias);
 		ret = MPATH_PR_DMMP_ERROR;
+		if (curmp)
+			vector_free(curmp);
+		if (pathvec)
+			vector_free(pathvec);
 		goto out;
 	}
 
@@ -285,6 +289,10 @@ int mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 	if (!curmp || !pathvec){
 		condlog (0, "%s: vector allocation failed.", alias);
 		ret = MPATH_PR_DMMP_ERROR;
+		if (curmp)
+			vector_free(curmp);
+		if (pathvec)
+			vector_free(pathvec);
 		goto out;
 	}
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 21/39] libmpathpersist: Pass sensebuffer as pointer
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (19 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 20/39] libmpathpersist: memory leak in mpath_persistent_reserve_(in, out)() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 22/39] libmpathpersist: Handle send error Hannes Reinecke
                   ` (18 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

It makes no sense to pass the entire sense buffer as a value,
we should rather pass the pointer to avoid exploding the stack.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmpathpersist/mpath_pr_ioctl.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index c85fd10..c37c1e4 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -24,7 +24,8 @@
 int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp *resp, int noisy);
 void mpath_format_readkeys(struct prin_resp *pr_buff, int len , int noisy);
 void mpath_format_readfullstatus(struct prin_resp *pr_buff, int len, int noisy);
-int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr, SenseData_t Sensedata, int noisy);
+int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr,
+			      SenseData_t *Sensedata, int noisy);
 void dumpHex(const char* str, int len, int no_ascii);
 int prout_do_scsi_ioctl( char * dev, int rq_servact, int rq_scope,
                 unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy);
@@ -113,7 +114,7 @@ retry :
 
 	condlog(2, "%s: Duration=%u (ms)", dev, io_hdr.duration);
 
-	status = mpath_translate_response(dev, io_hdr, Sensedata, noisy);
+	status = mpath_translate_response(dev, io_hdr, &Sensedata, noisy);
 	condlog(3, "%s: status = %d", dev, status);
 
 	if (status == MPATH_PR_SENSE_UNIT_ATTENTION && (retry > 0))
@@ -353,7 +354,7 @@ retry :
 	condlog(2, "%s: duration = %u (ms)", dev, io_hdr.duration);
 	condlog(2, "%s: persistent reservation in: requested %d bytes but got %d bytes)", dev, mx_resp_len, got);
 
-	status = mpath_translate_response(dev, io_hdr, Sensedata, noisy);
+	status = mpath_translate_response(dev, io_hdr, &Sensedata, noisy);
 
 	if (status == MPATH_PR_SENSE_UNIT_ATTENTION && (retry > 0))
 	{
@@ -398,26 +399,25 @@ out:
 	return status;
 }
 
-int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr, SenseData_t Sensedata, int noisy)
+int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr,
+			      SenseData_t *Sensedata, int noisy)
 {
-	condlog(3, "%s: status driver:%02x host:%02x scsi:%02x", dev, 
-			io_hdr.driver_status, io_hdr.host_status ,io_hdr.status);
+	condlog(3, "%s: status driver:%02x host:%02x scsi:%02x", dev,
+		io_hdr.driver_status, io_hdr.host_status, io_hdr.status);
 	io_hdr.status &= 0x7e;
-	if ((0 == io_hdr.status) && (0 == io_hdr.host_status) &&
-			(0 == io_hdr.driver_status))
-	{
+	if ((0 == io_hdr.status) &&
+	    (0 == io_hdr.host_status) &&
+	    (0 == io_hdr.driver_status))
 		return MPATH_PR_SUCCESS;
-	}
 
-	switch(io_hdr.status)
-	{
+	switch(io_hdr.status) {
 		case SAM_STAT_GOOD:
 			break;
 		case SAM_STAT_CHECK_CONDITION:
-			condlog(2, "%s: Sense_Key=%02x, ASC=%02x ASCQ=%02x", dev,
-					Sensedata.Sense_Key, Sensedata.ASC, Sensedata.ASCQ);
-			switch(Sensedata.Sense_Key)
-			{
+			condlog(2, "%s: Sense_Key=%02x, ASC=%02x ASCQ=%02x",
+				dev, Sensedata->Sense_Key,
+				Sensedata->ASC, Sensedata->ASCQ);
+			switch(Sensedata->Sense_Key) {
 				case NO_SENSE:
 					return MPATH_PR_NO_SENSE;
 				case RECOVERED_ERROR:
@@ -450,8 +450,7 @@ int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr, SenseData_t S
 			return  MPATH_PR_OTHER;
 	}
 
-	switch(io_hdr.host_status)
-	{
+	switch(io_hdr.host_status) {
 		case DID_OK :
 			break;
 		default :
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 22/39] libmpathpersist: Handle send error
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (20 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 21/39] libmpathpersist: Pass sensebuffer as pointer Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 23/39] libmultipath: missing break in hds prioritizer Hannes Reinecke
                   ` (17 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmpathpersist/mpath_updatepr.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c
index bda8991..0529d13 100644
--- a/libmpathpersist/mpath_updatepr.c
+++ b/libmpathpersist/mpath_updatepr.c
@@ -33,10 +33,14 @@ int update_prflag(char * arg1, char * arg2, int noisy)
 
 	snprintf(str,sizeof(str),"map %s %s", arg1, arg2);
 	condlog (2, "%s: pr flag message=%s", arg1, str);
-	send_packet(fd, str);
+	if (send_packet(fd, str) != 0) {
+		condlog(2, "%s: message=%s send error=%d", arg1, str, errno);
+		mpath_disconnect(fd);
+		return -2;
+	}
 	ret = recv_packet(fd, &reply, DEFAULT_REPLY_TIMEOUT);
 	if (ret < 0) {
-		condlog(2, "%s: message=%s error=%d", arg1, str, errno);
+		condlog(2, "%s: message=%s recv error=%d", arg1, str, errno);
 		ret = -2;
 	} else {
 		condlog (2, "%s: message=%s reply=%s", arg1, str, reply);
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 23/39] libmultipath: missing break in hds prioritizer
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (21 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 22/39] libmpathpersist: Handle send error Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 24/39] kpartx: fixup fd leakage in lopart.c Hannes Reinecke
                   ` (16 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/prioritizers/hds.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 8043b5b..db8c4c0 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -151,6 +151,7 @@ int hds_modular_prio (const char *dev, int fd)
 			return 0;
 			break;
 		}
+		break;
 	case '1': case '3': case '5': case '7': case '9':
 		switch (ldev[3]) {
 		case '0': case '2': case '4': case '6': case '8': case 'A': case 'C': case 'E':
@@ -162,6 +163,7 @@ int hds_modular_prio (const char *dev, int fd)
 			return 1;
 			break;
 		}
+		break;
 	}
 	return -1;
 }
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 24/39] kpartx: fixup fd leakage in lopart.c
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (22 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 23/39] libmultipath: missing break in hds prioritizer Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 25/39] kpartx: Memory leak in get_hotplug_device() Hannes Reinecke
                   ` (15 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We're leaking filedescriptors when trying to figure out the loop
device.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/lopart.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 39add44..e7f6090 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -154,13 +154,15 @@ find_unused_loop_device (void)
 	while (next_loop_dev == NULL) {
 		if (stat("/dev/loop-control", &statbuf) == 0 &&
 		    S_ISCHR(statbuf.st_mode)) {
-			fd = open("/dev/loop-control", O_RDWR);
-			if (fd < 0)
+			int next_loop_fd;
+
+			next_loop_fd = open("/dev/loop-control", O_RDWR);
+			if (next_loop_fd < 0)
 				return NULL;
-			next_loop = ioctl(fd, LOOP_CTL_GET_FREE);
+			next_loop = ioctl(next_loop_fd, LOOP_CTL_GET_FREE);
+			close(next_loop_fd);
 			if (next_loop < 0)
 				return NULL;
-			close(fd);
 		}
 
 		sprintf(dev, "/dev/loop%d", next_loop);
@@ -173,11 +175,8 @@ find_unused_loop_device (void)
 
 				if(ioctl (fd, LOOP_GET_STATUS, &loopinfo) == 0)
 					someloop++;		/* in use */
-
-				else if (errno == ENXIO) {
-					close (fd);
+				else if (errno == ENXIO)
 					next_loop_dev = xstrdup(dev);
-				}
 
 				close (fd);
 			}
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 25/39] kpartx: Memory leak in get_hotplug_device()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (23 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 24/39] kpartx: fixup fd leakage in lopart.c Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 26/39] kpartx: fixup filedescriptor leak in set_loop() Hannes Reinecke
                   ` (14 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We allocate a string for 'device', but never free it on error.
And the string 'mapname' is never freed at all.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/kpartx.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index e8c35d4..e1ebc96 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -178,16 +178,21 @@ get_hotplug_device(void)
 	len = strlen(mapname);
 
 	/* Dirname + mapname + \0 */
-	if (!(device = (char *)malloc(sizeof(char) * (off + len + 1))))
+	if (!(device = (char *)malloc(sizeof(char) * (off + len + 1)))) {
+		free(mapname);
 		return NULL;
+	}
 
 	/* Create new device name. */
 	snprintf(device, off + 1, "%s", devname);
 	snprintf(device + off, len + 1, "%s", mapname);
 
-	if (strlen(device) != (off + len))
+	if (strlen(device) != (off + len)) {
+		free(device);
+		free(mapname);
 		return NULL;
-
+	}
+	free(mapname);
 	return device;
 }
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 26/39] kpartx: fixup filedescriptor leak in set_loop()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (24 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 25/39] kpartx: Memory leak in get_hotplug_device() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 27/39] kpartx: do not use 'const' for mapname in get_hotplug_device() Hannes Reinecke
                   ` (13 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Upon error 'ffd' was never closed.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/lopart.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index e7f6090..5495e27 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -252,6 +252,7 @@ set_loop (const char *device, const char *file, int offset, int *loopro)
 	}
 
 	if ((fd = open (device, mode)) < 0) {
+		close(ffd);
 		perror (device);
 		return 1;
 	}
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 27/39] kpartx: do not use 'const' for mapname in get_hotplug_device()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (25 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 26/39] kpartx: fixup filedescriptor leak in set_loop() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 28/39] kpartx: Handle errors from lseek() Hannes Reinecke
                   ` (12 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/kpartx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index e1ebc96..4de13fa 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -150,7 +150,7 @@ static char *
 get_hotplug_device(void)
 {
 	unsigned int major, minor, off, len;
-	const char *mapname;
+	char *mapname;
 	char *devname = NULL;
 	char *device = NULL;
 	char *var = NULL;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 28/39] kpartx: Handle errors from lseek()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (26 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 27/39] kpartx: do not use 'const' for mapname in get_hotplug_device() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 29/39] kpartx/devmapper.c: Remove unused 'next' argument Hannes Reinecke
                   ` (11 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/gpt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kpartx/gpt.c b/kpartx/gpt.c
index 5a54970..3dac605 100644
--- a/kpartx/gpt.c
+++ b/kpartx/gpt.c
@@ -206,9 +206,10 @@ read_lba(int fd, uint64_t lba, void *buffer, size_t bytes)
 	int sector_size = get_sector_size(fd);
 	off_t offset = lba * sector_size;
 	uint64_t lastlba;
-        ssize_t bytesread;
+	ssize_t bytesread;
 
-	lseek(fd, offset, SEEK_SET);
+	if (lseek(fd, offset, SEEK_SET) < 0)
+		return 0;
 	bytesread = read(fd, buffer, bytes);
 
 	lastlba = last_lba(fd);
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 29/39] kpartx/devmapper.c: Remove unused 'next' argument
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (27 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 28/39] kpartx: Handle errors from lseek() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 30/39] kpartx: use fstat() when reading sysfs attributes Hannes Reinecke
                   ` (10 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/devmapper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index c5ba39d..382f511 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -311,7 +311,6 @@ dm_get_map(int major, int minor, char * outparams)
 {
 	int r = 1;
 	struct dm_task *dmt;
-	void *next = NULL;
 	uint64_t start, length;
 	char *target_type = NULL;
 	char *params = NULL;
@@ -327,8 +326,8 @@ dm_get_map(int major, int minor, char * outparams)
 		goto out;
 
 	/* Fetch 1st target */
-	next = dm_get_next_target(dmt, next, &start, &length,
-				  &target_type, &params);
+	dm_get_next_target(dmt, NULL, &start, &length,
+			   &target_type, &params);
 
 	if (snprintf(outparams, PARAMS_SIZE, "%s", params) <= PARAMS_SIZE)
 		r = 0;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 30/39] kpartx: use fstat() when reading sysfs attributes
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (28 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 29/39] kpartx/devmapper.c: Remove unused 'next' argument Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 31/39] kpartx/lopart.c: use fstat() when reading device nodes Hannes Reinecke
                   ` (9 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Coverity pointed out that we shouldn't be using stat()/open(),
as the file might have vanished or changed in between those
two calls. So modify it to open()/fstat() instead.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/lopart.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 5495e27..d4a2ab4 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -117,14 +117,16 @@ find_loop_by_file (const char * filename)
 			continue;
 		sprintf(dev, "/dev/%s", dent->d_name);
 
-		if (stat (dev, &statbuf) != 0 ||
-		    !S_ISBLK(statbuf.st_mode))
-			continue;
-
 		fd = open (dev, O_RDONLY);
 		if (fd < 0)
 			break;
 
+		if (fstat (fd, &statbuf) != 0 ||
+		    !S_ISBLK(statbuf.st_mode)) {
+			close (fd);
+			continue;
+		}
+
 		if (ioctl (fd, LOOP_GET_STATUS, &loopinfo) != 0) {
 			close (fd);
 			continue;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 31/39] kpartx/lopart.c: use fstat() when reading device nodes
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (29 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 30/39] kpartx: use fstat() when reading sysfs attributes Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 32/39] libmultipath/sysfs.c: always terminate value from sysfs_attr_get_value() Hannes Reinecke
                   ` (8 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Coverity pointed out that we shouldn't be using stat()/open(),
as the file might have vanished or changed in between those
two calls. So modify it to open()/fstat() instead.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 kpartx/lopart.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index d4a2ab4..f29cfc1 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -169,19 +169,18 @@ find_unused_loop_device (void)
 
 		sprintf(dev, "/dev/loop%d", next_loop);
 
-		if (stat (dev, &statbuf) == 0 && S_ISBLK(statbuf.st_mode)) {
-			somedev++;
-			fd = open (dev, O_RDONLY);
-
-			if (fd >= 0) {
-
+		fd = open (dev, O_RDONLY);
+		if (fd >= 0) {
+			if (fstat (fd, &statbuf) == 0 &&
+			    S_ISBLK(statbuf.st_mode)) {
+				somedev++;
 				if(ioctl (fd, LOOP_GET_STATUS, &loopinfo) == 0)
 					someloop++;		/* in use */
 				else if (errno == ENXIO)
 					next_loop_dev = xstrdup(dev);
 
-				close (fd);
 			}
+			close (fd);
 
 			/* continue trying as long as devices exist */
 			continue;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 32/39] libmultipath/sysfs.c: always terminate value from sysfs_attr_get_value()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (30 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 31/39] kpartx/lopart.c: use fstat() when reading device nodes Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 33/39] multipathd: missing mpath_disconnect() in uxclnt() Hannes Reinecke
                   ` (7 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Coverity complained that the returned value is not always NULL
terminated.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/sysfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index a74de9f..2c0b591 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -86,16 +86,17 @@ ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
 	if (size < 0) {
 		condlog(4, "read from %s failed: %s", devpath, strerror(errno));
 		size = -errno;
+		value[0] = '\0';
 	} else if (size == value_len) {
+		value[size - 1] = '\0';
 		condlog(4, "overflow while reading from %s", devpath);
 		size = 0;
 	} else {
 		value[size] = '\0';
+		size = strchop(value);
 	}
 
 	close(fd);
-	if (size > 0)
-		size = strchop(value);
 	return size;
 }
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 33/39] multipathd: missing mpath_disconnect() in uxclnt()
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (31 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 32/39] libmultipath/sysfs.c: always terminate value from sysfs_attr_get_value() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 34/39] multipathd: pid fd resource leak Hannes Reinecke
                   ` (6 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We should be calling mpath_disconnect() in uxclnt().
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 multipathd/uxclnt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
index 06c1bf8..37afaac 100644
--- a/multipathd/uxclnt.c
+++ b/multipathd/uxclnt.c
@@ -135,6 +135,6 @@ int uxclnt(char * inbuf, unsigned int timeout)
 		process_req(fd, inbuf, timeout);
 	else
 		process(fd, timeout);
-
+	mpath_disconnect(fd);
 	return 0;
 }
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 34/39] multipathd: pid fd resource leak
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (32 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 33/39] multipathd: missing mpath_disconnect() in uxclnt() Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 35/39] libmultipath: mark return value from dup() as unused Hannes Reinecke
                   ` (5 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We open a PID filedescriptor, but never close them.
While this is not an error per se coverity complains here.
So be a good guy and close the PID fd properly.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 multipathd/main.c    | 6 +++++-
 multipathd/pidfile.c | 6 +++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 0eadd70..8285f0d 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2085,6 +2085,7 @@ child (void * param)
 	unsigned long checkint;
 #endif
 	int rc;
+	int pid_fd = -1;
 	char *envp;
 
 	mlockall(MCL_CURRENT | MCL_FUTURE);
@@ -2101,7 +2102,8 @@ child (void * param)
 		log_thread_start(&log_attr);
 		pthread_attr_destroy(&log_attr);
 	}
-	if (pidfile_create(DEFAULT_PIDFILE, daemon_pid)) {
+	pid_fd = pidfile_create(DEFAULT_PIDFILE, daemon_pid);
+	if (pid_fd < 0) {
 		condlog(1, "failed to create pidfile");
 		if (logsink == 1)
 			log_thread_stop();
@@ -2307,6 +2309,8 @@ failed:
 #ifdef USE_SYSTEMD
 	sd_notify(0, "ERRNO=1");
 #endif
+	if (pid_fd >= 0)
+		close(pid_fd);
 	exit(1);
 }
 
diff --git a/multipathd/pidfile.c b/multipathd/pidfile.c
index e3fb896..e911358 100644
--- a/multipathd/pidfile.c
+++ b/multipathd/pidfile.c
@@ -22,7 +22,7 @@ int pidfile_create(const char *pidFile, pid_t pid)
 		       (S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))) < 0) {
 		condlog(0, "Cannot open pidfile [%s], error was [%s]",
 			pidFile, strerror(errno));
-		return 1;
+		return -errno;
 	}
 	lock.l_type = F_WRLCK;
 	lock.l_start = 0;
@@ -60,8 +60,8 @@ int pidfile_create(const char *pidFile, pid_t pid)
 			"error was [%s]", pidFile, strerror(errno));
 		goto fail;
 	}
-	return 0;
+	return fd;
 fail:
 	close(fd);
-	return 1;
+	return -errno;
 }
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 35/39] libmultipath: mark return value from dup() as unused
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (33 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 34/39] multipathd: pid fd resource leak Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 36/39] libmultipath: Ignore errors from fcntl() in directio.c Hannes Reinecke
                   ` (4 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

We are redirecting stderr, and can safely ignore any
errors from dup() here. So mark the return value as
unused.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/callout.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libmultipath/callout.c b/libmultipath/callout.c
index d671b0c..4d1b067 100644
--- a/libmultipath/callout.c
+++ b/libmultipath/callout.c
@@ -76,8 +76,10 @@ int execute_program(char *path, char *value, int len)
 		/* Ignore writes to stderr */
 		null_fd = open("/dev/null", O_WRONLY);
 		if (null_fd > 0) {
+			int err_fd __attribute__ ((unused));
+
 			close(STDERR_FILENO);
-			retval = dup(null_fd);
+			err_fd = dup(null_fd);
 			close(null_fd);
 		}
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 36/39] libmultipath: Ignore errors from fcntl() in directio.c
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (34 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 35/39] libmultipath: mark return value from dup() as unused Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 37/39] libmultipath: add spin_lock in tur.c Hannes Reinecke
                   ` (3 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Mark the return value from fcntl() as unused to avoid compiler
warnings.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/checkers/directio.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c
index 46fe6a7..94bf8f7 100644
--- a/libmultipath/checkers/directio.c
+++ b/libmultipath/checkers/directio.c
@@ -104,9 +104,11 @@ void libcheck_free (struct checker * c)
 
 	if (ct->reset_flags) {
 		if ((flags = fcntl(c->fd, F_GETFL)) >= 0) {
+			int ret __attribute__ ((unused));
+
 			flags &= ~O_DIRECT;
 			/* No point in checking for errors */
-			fcntl(c->fd, F_SETFL, flags);
+			ret = fcntl(c->fd, F_SETFL, flags);
 		}
 	}
 
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 37/39] libmultipath: add spin_lock in tur.c
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (35 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 36/39] libmultipath: Ignore errors from fcntl() in directio.c Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 38/39] libmultipath/devmapper.c: fixup possible buffer overflow Hannes Reinecke
                   ` (2 subsequent siblings)
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

When failing to start the pthread we still should be taking
the spin lock; there might be other threads pending.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/checkers/tur.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index bd7372d..2edc8ad 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -331,9 +331,11 @@ libcheck_check (struct checker * c)
 		setup_thread_attr(&attr, 32 * 1024, 1);
 		r = pthread_create(&ct->thread, &attr, tur_thread, ct);
 		if (r) {
+			pthread_spin_lock(&ct->hldr_lock);
+			ct->holders--;
+			pthread_spin_unlock(&ct->hldr_lock);
 			pthread_mutex_unlock(&ct->lock);
 			ct->thread = 0;
-			ct->holders--;
 			condlog(3, "%d:%d: failed to start tur thread, using"
 				" sync mode", TUR_DEVT(ct));
 			return tur_check(c->fd, c->timeout, c->message);
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 38/39] libmultipath/devmapper.c: fixup possible buffer overflow
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (36 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 37/39] libmultipath: add spin_lock in tur.c Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-16  9:47 ` [PATCH 39/39] libmultipath/discovery.c: Fixup possible range overflow Hannes Reinecke
  2016-06-20  7:09 ` [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

Fixup possible buffer overflow in dm_reassign_table().
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/devmapper.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 0223e18..926d2f5 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1434,15 +1434,18 @@ out:
 void dm_reassign_deps(char *table, char *dep, char *newdep)
 {
 	char *p, *n;
-	char newtable[PARAMS_SIZE];
+	char *newtable;
 
-	strcpy(newtable, table);
+	newtable = strdup(table);
+	if (!newtable)
+		return;
 	p = strstr(newtable, dep);
 	n = table + (p - newtable);
 	strcpy(n, newdep);
 	n += strlen(newdep);
 	p += strlen(dep);
 	strcat(n, p);
+	free(newtable);
 }
 
 int dm_reassign_table(const char *name, char *old, char *new)
@@ -1451,7 +1454,7 @@ int dm_reassign_table(const char *name, char *old, char *new)
 	uint64_t start, length;
 	struct dm_task *dmt, *reload_dmt;
 	char *target, *params = NULL;
-	char buff[PARAMS_SIZE];
+	char *buff;
 	void *next = NULL;
 
 	if (!(dmt = dm_task_create(DM_DEVICE_TABLE)))
@@ -1472,8 +1475,12 @@ int dm_reassign_table(const char *name, char *old, char *new)
 	do {
 		next = dm_get_next_target(dmt, next, &start, &length,
 					  &target, &params);
-		memset(buff, 0, PARAMS_SIZE);
-		strcpy(buff, params);
+		buff = strdup(params);
+		if (!buff) {
+			condlog(3, "%s: failed to replace target %s, "
+				"out of memory", name, target);
+			goto out_reload;
+		}
 		if (strcmp(target, TGT_MPATH) && strstr(params, old)) {
 			condlog(3, "%s: replace target %s %s",
 				name, target, buff);
@@ -1483,6 +1490,7 @@ int dm_reassign_table(const char *name, char *old, char *new)
 			modified++;
 		}
 		dm_task_add_target(reload_dmt, start, length, target, buff);
+		free(buff);
 	} while (next);
 
 	if (modified) {
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 39/39] libmultipath/discovery.c: Fixup possible range overflow
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (37 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 38/39] libmultipath/devmapper.c: fixup possible buffer overflow Hannes Reinecke
@ 2016-06-16  9:47 ` Hannes Reinecke
  2016-06-20  7:09 ` [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
  39 siblings, 0 replies; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-16  9:47 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: Hannes Reinecke, dm-devel

When checking the maximum value for dev_loss_tmo we need to
use a larger type, otherwise the comparison will always be false.
Found by coverity.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 libmultipath/discovery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 5d5441d..126a54f 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -670,7 +670,7 @@ sysfs_set_scsi_tmo (struct multipath *mpp)
 	int dev_loss_tmo = mpp->dev_loss;
 
 	if (mpp->no_path_retry > 0) {
-		int no_path_retry_tmo = mpp->no_path_retry * conf->checkint;
+		uint64_t no_path_retry_tmo = mpp->no_path_retry * conf->checkint;
 
 		if (no_path_retry_tmo > MAX_DEV_LOSS_TMO)
 			no_path_retry_tmo = MAX_DEV_LOSS_TMO;
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [PATCH 00/39] multipath: coverity fixes
  2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
                   ` (38 preceding siblings ...)
  2016-06-16  9:47 ` [PATCH 39/39] libmultipath/discovery.c: Fixup possible range overflow Hannes Reinecke
@ 2016-06-20  7:09 ` Hannes Reinecke
  2016-06-20  7:32   ` Christophe Varoqui
  39 siblings, 1 reply; 42+ messages in thread
From: Hannes Reinecke @ 2016-06-20  7:09 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: dm-devel

On 06/16/2016 11:47 AM, Hannes Reinecke wrote:
> Hi all,
> 
> here's a bunch of fixes from a recent coverity run.
> Nothing critical, mostly memory leaks and possible
> buffer overflows. And the usual missing return value
> check.
> 
> As usual, comments and reviews are welcome.
> 
Ping?
I've got another patchset (converting config accesses to RCU)
which sort of relies on this one...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 00/39] multipath: coverity fixes
  2016-06-20  7:09 ` [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
@ 2016-06-20  7:32   ` Christophe Varoqui
  0 siblings, 0 replies; 42+ messages in thread
From: Christophe Varoqui @ 2016-06-20  7:32 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: device-mapper development


[-- Attachment #1.1: Type: text/plain, Size: 865 bytes --]

I pulled the coverity branch today.
Thanks.

On Mon, Jun 20, 2016 at 9:09 AM, Hannes Reinecke <hare@suse.de> wrote:

> On 06/16/2016 11:47 AM, Hannes Reinecke wrote:
> > Hi all,
> >
> > here's a bunch of fixes from a recent coverity run.
> > Nothing critical, mostly memory leaks and possible
> > buffer overflows. And the usual missing return value
> > check.
> >
> > As usual, comments and reviews are welcome.
> >
> Ping?
> I've got another patchset (converting config accesses to RCU)
> which sort of relies on this one...
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke                Teamlead Storage & Networking
> hare@suse.de                                   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
>

[-- Attachment #1.2: Type: text/html, Size: 1447 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2016-06-20  7:32 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-16  9:47 [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
2016-06-16  9:47 ` [PATCH 01/39] multipathd: correctly free refwwid in cli_add_map() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 02/39] libmultipath: fixup string copy and comparison Hannes Reinecke
2016-06-16  9:47 ` [PATCH 03/39] libmultipath: fixup uninitialized return value in dm_reassign_table() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 04/39] libmultipath: free pathgroup in group_by_prio() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 05/39] libmultipath: memory leak in remove_feature() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 06/39] libmultipath: memory leak in add_map_without_path() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 07/39] libmultipath: missing NULL check in set_pgfailback() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 08/39] libmultipath: remove stale references to fd_ep in uevent_listen() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 09/39] libmultipath/devmapper.c: Remove unused 'next' argument Hannes Reinecke
2016-06-16  9:47 ` [PATCH 10/39] libmultipath/alias.c: check return value from strchr() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 11/39] libmultipath/devmapper.c: check for errors from sscanf() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 12/39] libmultipath/uevent.c: handle errors from setsockopt() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 13/39] libmultipath/discovery.c: handle errors from sscanf() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 14/39] libmultipath: drop unused variable 'diop' in libsg.c Hannes Reinecke
2016-06-16  9:47 ` [PATCH 15/39] libmultipath: use fstat() when reading sysfs attributes Hannes Reinecke
2016-06-16  9:47 ` [PATCH 16/39] libmultipath: fixup possible buffer overflow in alua_rtpg.c Hannes Reinecke
2016-06-16  9:47 ` [PATCH 17/39] libmultipath: memory leak in iet.c Hannes Reinecke
2016-06-16  9:47 ` [PATCH 18/39] libmpathcmd: limit size of the CLI buffer Hannes Reinecke
2016-06-16  9:47 ` [PATCH 19/39] libmpathpersist: Fix possible overrun in devt2devname() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 20/39] libmpathpersist: memory leak in mpath_persistent_reserve_(in, out)() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 21/39] libmpathpersist: Pass sensebuffer as pointer Hannes Reinecke
2016-06-16  9:47 ` [PATCH 22/39] libmpathpersist: Handle send error Hannes Reinecke
2016-06-16  9:47 ` [PATCH 23/39] libmultipath: missing break in hds prioritizer Hannes Reinecke
2016-06-16  9:47 ` [PATCH 24/39] kpartx: fixup fd leakage in lopart.c Hannes Reinecke
2016-06-16  9:47 ` [PATCH 25/39] kpartx: Memory leak in get_hotplug_device() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 26/39] kpartx: fixup filedescriptor leak in set_loop() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 27/39] kpartx: do not use 'const' for mapname in get_hotplug_device() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 28/39] kpartx: Handle errors from lseek() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 29/39] kpartx/devmapper.c: Remove unused 'next' argument Hannes Reinecke
2016-06-16  9:47 ` [PATCH 30/39] kpartx: use fstat() when reading sysfs attributes Hannes Reinecke
2016-06-16  9:47 ` [PATCH 31/39] kpartx/lopart.c: use fstat() when reading device nodes Hannes Reinecke
2016-06-16  9:47 ` [PATCH 32/39] libmultipath/sysfs.c: always terminate value from sysfs_attr_get_value() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 33/39] multipathd: missing mpath_disconnect() in uxclnt() Hannes Reinecke
2016-06-16  9:47 ` [PATCH 34/39] multipathd: pid fd resource leak Hannes Reinecke
2016-06-16  9:47 ` [PATCH 35/39] libmultipath: mark return value from dup() as unused Hannes Reinecke
2016-06-16  9:47 ` [PATCH 36/39] libmultipath: Ignore errors from fcntl() in directio.c Hannes Reinecke
2016-06-16  9:47 ` [PATCH 37/39] libmultipath: add spin_lock in tur.c Hannes Reinecke
2016-06-16  9:47 ` [PATCH 38/39] libmultipath/devmapper.c: fixup possible buffer overflow Hannes Reinecke
2016-06-16  9:47 ` [PATCH 39/39] libmultipath/discovery.c: Fixup possible range overflow Hannes Reinecke
2016-06-20  7:09 ` [PATCH 00/39] multipath: coverity fixes Hannes Reinecke
2016-06-20  7:32   ` Christophe Varoqui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.