libmultipath: Fix possible string overflow

basenamecpy() and devt2devname() need to be passed in the
length of the string into which the modified value should
be copied. Otherwise a string overflow can be induced.

Signed-off-by: Hannes Reinecke <hare@suse.de>

6 files changed, 116 insertions, 73 deletions

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index ce559e4..f568287 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -630,9 +630,13 @@ get_refwwid (char * dev, enum devtypes dev_type, vector pathvec)
 		return NULL;
 
 	if (dev_type == DEV_DEVNODE) {
-		basenamecpy(dev, buff);
+		if (basenamecpy(dev, buff, FILE_NAME_SIZE) == 0) {
+			condlog(1, "basename failed for '%s' (%s)",
+				dev, buff);
+			return NULL;
+		}
+
 		pp = find_path_by_dev(pathvec, buff);
-		
 		if (!pp) {
 			pp = alloc_path();
 
@@ -656,9 +660,8 @@ get_refwwid (char * dev, enum devtypes dev_type, vector pathvec)
 	if (dev_type == DEV_DEVT) {
 		strchop(dev);
 		pp = find_path_by_devt(pathvec, dev);
-		
 		if (!pp) {
-			if (devt2devname(buff, dev))
+			if (devt2devname(buff, FILE_NAME_SIZE, dev))
 				return NULL;
 
 			pp = alloc_path();
@@ -670,7 +673,7 @@ get_refwwid (char * dev, enum devtypes dev_type, vector pathvec)
 
 			if (pathinfo(pp, conf->hwtable, DI_SYSFS | DI_WWID))
 				return NULL;
-			
+
 			if (store_path(pathvec, pp)) {
 				free_path(pp);
 				return NULL;
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index aa25cc4..0456b6d 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -329,62 +329,6 @@ opennode (char * dev, int mode)
 	return open(devpath, mode);
 }
 
-extern int
-devt2devname (char *devname, char *devt)
-{
-	FILE *fd;
-	unsigned int tmpmaj, tmpmin, major, minor;
-	char dev[FILE_NAME_SIZE];
-	char block_path[FILE_NAME_SIZE];
-	struct stat statbuf;
-
-	memset(block_path, 0, FILE_NAME_SIZE);
-	if (sscanf(devt, "%u:%u", &major, &minor) != 2) {
-		condlog(0, "Invalid device number %s", devt);
-		return 1;
-	}
-
-	if (!(fd = fopen("/proc/partitions", "r"))) {
-		condlog(0, "Cannot open /proc/partitions");
-		return 1;
-	}
-
-	while (!feof(fd)) {
-		int r = fscanf(fd,"%u %u %*d %s",&tmpmaj, &tmpmin, dev);
-		if (!r) {
-			r = fscanf(fd,"%*s\n");
-			continue;
-		}
-		if (r != 3)
-			continue;
-
-		if ((major == tmpmaj) && (minor == tmpmin)) {
-			if (snprintf(block_path, FILE_NAME_SIZE, "/sys/block/%s", dev) >= FILE_NAME_SIZE) {
-				condlog(0, "device name %s is too long\n", dev);
-				fclose(fd);
-				return 1;
-			}
-			break;
-		}
-	}
-	fclose(fd);
-
-	if (strncmp(block_path,"/sys/block", 10))
-		return 1;
-
-	if (stat(block_path, &statbuf) < 0) {
-		condlog(0, "No sysfs entry for %s\n", block_path);
-		return 1;
-	}
-
-	if (S_ISDIR(statbuf.st_mode) == 0) {
-		condlog(0, "sysfs entry %s is not a directory\n", block_path);
-		return 1;
-	}
-	basenamecpy(block_path, devname);
-	return 0;
-}
-
 int
 do_inq(int sg_fd, int cmddt, int evpd, unsigned int pg_op,
        void *resp, int mx_resp_len)
@@ -570,7 +514,7 @@ scsi_sysfs_pathinfo (struct path * pp, struct sysfs_device * parent)
 	/*
 	 * host / bus / target / lun
 	 */
-	basenamecpy(parent->devpath, attr_path);
+	basenamecpy(parent->devpath, attr_path, FILE_NAME_SIZE);
 
 	sscanf(attr_path, "%i:%i:%i:%i",
 			&pp->sg_id.host_no,
@@ -629,7 +573,7 @@ ccw_sysfs_pathinfo (struct path * pp, struct sysfs_device * parent)
 	/*
 	 * host / bus / target / lun
 	 */
-	basenamecpy(parent->devpath, attr_path);
+	basenamecpy(parent->devpath, attr_path, FILE_NAME_SIZE);
 	pp->sg_id.lun = 0;
 	sscanf(attr_path, "%i.%i.%x",
 			&pp->sg_id.host_no,
@@ -653,7 +597,7 @@ cciss_sysfs_pathinfo (struct path * pp, struct sysfs_device * dev)
 	/*
 	 * host / bus / target / lun
 	 */
-	basenamecpy(dev->devpath, attr_path);
+	basenamecpy(dev->devpath, attr_path, FILE_NAME_SIZE);
 	pp->sg_id.lun = 0;
 	pp->sg_id.channel = 0;
 	sscanf(attr_path, "cciss!c%id%i",
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index c376702..0ba33d8 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -28,7 +28,6 @@ int sysfs_get_dev (struct sysfs_device * dev, char * buff, size_t len);
 int path_discovery (vector pathvec, struct config * conf, int flag);
 
 int do_tur (char *);
-int devt2devname (char *, char *);
 int path_offline (struct path *);
 int get_state (struct path * pp, int daemon);
 int pathinfo (struct path *, vector hwtable, int mask);
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 2b24885..f2ad630 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -6,8 +6,9 @@
 
 #include "debug.h"
 #include "memory.h"
-
-#define PARAMS_SIZE 255
+#include "checkers.h"
+#include "vector.h"
+#include "structs.h"
 
 void
 strchop(char *str)
@@ -18,10 +19,21 @@ strchop(char *str)
 	str[++i] = '\0';
 }
 
-void
-basenamecpy (char * str1, char * str2)
+int
+basenamecpy (char * str1, char * str2, int str2len)
 {
-	char *p = str1 + (strlen(str1) - 1);
+	char *p;
+
+	if (!str1 || !strlen(str1))
+		return 0;
+
+	if (strlen(str1) > str2len)
+		return 0;
+
+	if (!str2)
+		return 0;
+
+	p = str1 + (strlen(str1) - 1);
 
 	while (*--p != '/' && p != str1)
 		continue;
@@ -29,7 +41,10 @@ basenamecpy (char * str1, char * str2)
 	if (p != str1)
 		p++;
 
-	strcpy(str2, p);
+	strncpy(str2, p, str2len);
+	str2[str2len - 1] = '\0';
+	strchop(str2);
+	return strlen(str2);
 }
 
 int
@@ -136,3 +151,83 @@ void remove_trailing_chars(char *path, char c)
 		path[--len] = '\0';
 }
 
+extern int
+devt2devname (char *devname, int devname_len, char *devt)
+{
+	FILE *fd;
+	unsigned int tmpmaj, tmpmin, major, minor;
+	char dev[FILE_NAME_SIZE];
+	char block_path[PATH_SIZE];
+	struct stat statbuf;
+
+	memset(block_path, 0, sizeof(block_path));
+	if (sscanf(devt, "%u:%u", &major, &minor) != 2) {
+		condlog(0, "Invalid device number %s", devt);
+		return 1;
+	}
+
+	if (devname_len > FILE_NAME_SIZE)
+		devname_len = FILE_NAME_SIZE;
+
+	sprintf(block_path,"/sys/dev/block/%u:%u", major, minor);
+	if (stat(block_path, &statbuf) == 0) {
+		/* Newer kernels have /sys/dev/block */
+		if (S_ISLNK(statbuf.st_mode) &&
+		    readlink(block_path, dev, FILE_NAME_SIZE) > 0) {
+			char *p = strrchr(dev, '/');
+
+			if (!p) {
+				condlog(0, "No sysfs entry for %s\n",
+					block_path);
+				return 1;
+			}
+			p++;
+			strncpy(devname, p, devname_len);
+			return 0;
+		}
+	}
+	memset(block_path, 0, sizeof(block_path));
+
+	if (!(fd = fopen("/proc/partitions", "r"))) {
+		condlog(0, "Cannot open /proc/partitions");
+		return 1;
+	}
+
+	while (!feof(fd)) {
+		int r = fscanf(fd,"%u %u %*d %s",&tmpmaj, &tmpmin, dev);
+		if (!r) {
+			r = fscanf(fd,"%*s\n");
+			continue;
+		}
+		if (r != 3)
+			continue;
+
+		if ((major == tmpmaj) && (minor == tmpmin)) {
+			if (snprintf(block_path, sizeof(block_path),
+				     "/sys/block/%s", dev) >= sizeof(block_path)) {
+				condlog(0, "device name %s is too long\n", dev);
+				fclose(fd);
+				return 1;
+			}
+			break;
+		}
+	}
+	fclose(fd);
+
+	if (strncmp(block_path,"/sys/block", 10)) {
+		condlog(3, "device %s not found\n", dev);
+		return 1;
+	}
+
+	if (stat(block_path, &statbuf) < 0) {
+		condlog(0, "No sysfs entry for %s\n", block_path);
+		return 1;
+	}
+
+	if (S_ISDIR(statbuf.st_mode) == 0) {
+		condlog(0, "sysfs entry %s is not a directory\n", block_path);
+		return 1;
+	}
+	basenamecpy(block_path, devname, devname_len);
+	return 0;
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index ceab398..72de319 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -2,12 +2,13 @@
 #define _UTIL_H
 
 void strchop(char *);
-void basenamecpy (char * src, char * dst);
+int basenamecpy (char * src, char * dst, int);
 int filepresent (char * run);
 int get_word (char * sentence, char ** word);
 size_t strlcpy(char *dst, const char *src, size_t size);
 size_t strlcat(char *dst, const char *src, size_t size);
 void remove_trailing_chars(char *path, char c);
+int devt2devname (char *, int, char *);
 
 #define safe_sprintf(var, format, args...)	\
 	snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
diff --git a/multipath/main.c b/multipath/main.c
index 5d718e7..de50e65 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -132,7 +132,8 @@ update_paths (struct multipath * mpp)
 
 		vector_foreach_slot (pgp->paths, pp, j) {
 			if (!strlen(pp->dev)) {
-				if (devt2devname(pp->dev, pp->dev_t)) {
+				if (devt2devname(pp->dev, FILE_NAME_SIZE,
+						 pp->dev_t)) {
 					/*
 					 * path is not in sysfs anymore
 					 */