[dm-devel] [PATCH 3/6] check pgp to avoid NULL dereference in enable_group

[lixiaokeng]

If the pgp is NULL, this code will lead to coredump.
Here we check the pgp before dereference. And there
are some code enhance.

Signed-off-by: Lixiaokeng <lixiaokeng at huawei.com>
---
 libmultipath/prioritizers/alua.c |  4 ++++
 libmultipath/prioritizers/emc.c  |  2 +-
 libmultipath/prioritizers/hds.c  | 19 +++++++++++--------
 libmultipath/structs_vec.c       |  4 +++-
 multipathd/main.c                |  5 +++++
 5 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 0ab06e2b..a6307d69 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -131,15 +131,19 @@ int getprio (struct path * pp, char * args, unsigned int timeout)
 		switch(-rc) {
 		case ALUA_PRIO_NOT_SUPPORTED:
 			condlog(0, "%s: alua not supported", pp->dev);
+			rc = 0;
 			break;
 		case ALUA_PRIO_RTPG_FAILED:
 			condlog(0, "%s: couldn't get target port group", pp->dev);
+			rc = 0;
 			break;
 		case ALUA_PRIO_GETAAS_FAILED:
 			condlog(0, "%s: couldn't get asymmetric access state", pp->dev);
+			rc = 0;
 			break;
 		case ALUA_PRIO_TPGS_FAILED:
 			condlog(3, "%s: couldn't get supported alua states", pp->dev);
+			rc = 0;
 			break;
 		}
 	}
diff --git a/libmultipath/prioritizers/emc.c b/libmultipath/prioritizers/emc.c
index 3b63cca0..c40f88a0 100644
--- a/libmultipath/prioritizers/emc.c
+++ b/libmultipath/prioritizers/emc.c
@@ -19,7 +19,7 @@ int emc_clariion_prio(const char *dev, int fd, unsigned int timeout)
 	unsigned char inqCmdBlk[INQUIRY_CMDLEN] = {INQUIRY_CMD, 1, 0xC0, 0,
 						sizeof(sense_buffer), 0};
 	struct sg_io_hdr io_hdr;
-	int ret = PRIO_UNDEF;
+	int ret = 0;

 	memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
 	memset(&sense_buffer, 0, 128);
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 88cac5f0..634ae73a 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -98,10 +98,11 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)
 	unsigned char *inqBuffp = inqBuff;
 	unsigned char sense_buffer[32];
 	sg_io_hdr_t io_hdr;
+        int ret = 0;

 	if ((ioctl (fd, SG_GET_VERSION_NUM, &k) < 0) || (k < 30000)) {
 		pp_hds_log(0, "can't use SG ioctl interface");
-		return -1;
+		goto out;
 	}

 	memset (&io_hdr, 0, sizeof (sg_io_hdr_t));
@@ -118,11 +119,11 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)

 	if (ioctl (fd, SG_IO, &io_hdr) < 0) {
 		pp_hds_log(0, "SG_IO error");
-		return -1;
+		goto out;
 	}
 	if ((io_hdr.info & SG_INFO_OK_MASK) != SG_INFO_OK) {
 		pp_hds_log(0, "SCSI error");
-		return -1;
+		goto out;
 	}

 	snprintf (vendor, 9, "%.8s", inqBuffp + 8);
@@ -144,11 +145,11 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)
 		switch (ldev[3]) {
 		case '0': case '2': case '4': case '6': case '8': case 'A': case 'C': case 'E':
 			pp_hds_log(4, "CTRL EVEN, LDEV EVEN, PRIO 1");
-			return 1;
+			ret = 1;
 			break;
 		case '1': case '3': case '5': case '7': case '9': case 'B': case 'D': case 'F':
 			pp_hds_log(4, "CTRL EVEN, LDEV ODD, PRIO 0");
-			return 0;
+			ret = 0;
 			break;
 		}
 		break;
@@ -156,16 +157,18 @@ int hds_modular_prio (const char *dev, int fd, unsigned int timeout)
 		switch (ldev[3]) {
 		case '0': case '2': case '4': case '6': case '8': case 'A': case 'C': case 'E':
 			pp_hds_log(4, "CTRL ODD, LDEV EVEN, PRIO 0");
-			return 0;
+			ret = 0;
 			break;
 		case '1': case '3': case '5': case '7': case '9': case 'B': case 'D': case 'F':
 			pp_hds_log(4, "CTRL ODD, LDEV ODD, PRIO 1");
-			return 1;
+			ret = 1;
 			break;
 		}
 		break;
 	}
-	return -1;
+
+out:
+	return ret;
 }

 int getprio (struct path * pp, __attribute__((unused)) char *args,
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 5a73d4ce..51696e36 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -35,8 +35,10 @@ int update_mpp_paths(struct multipath *mpp, vector pathvec)
 		return 0;

 	if (!mpp->paths &&
-	    !(mpp->paths = vector_alloc()))
+	    !(mpp->paths = vector_alloc())) {
+		condlog(2, "mpp->paths alloc failed");
 		return 1;
+	}

 	vector_foreach_slot (mpp->pg, pgp, i) {
 		vector_foreach_slot (pgp->paths, pp, j) {
diff --git a/multipathd/main.c b/multipathd/main.c
index a4abbb27..13cb4dec 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1725,6 +1725,11 @@ enable_group(struct path * pp)

 	pgp = VECTOR_SLOT(pp->mpp->pg, pp->pgindex - 1);

+	if (!pgp) {
+		condlog(2, "%s: pgp is NULL", pp->mpp->alias);
+		return;
+	}
+
 	if (pgp->status == PGSTATE_DISABLED) {
 		condlog(2, "%s: enable group #%i", pp->mpp->alias, pp->pgindex);
 		dm_enablegroup(pp->mpp->alias, pp->pgindex);
--