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>

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index ce559e4..f568287 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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
                                         */