--- libmultipath/discovery.c | 50 ++++++--------------- libmultipath/sysfs.c | 108 ++++++++++++++++++----------------------------- libmultipath/sysfs.h | 3 - multipathd/main.c | 7 +-- 4 files changed, 62 insertions(+), 106 deletions(-) Index: multipath-tools/libmultipath/discovery.c =================================================================== --- multipath-tools.orig/libmultipath/discovery.c +++ multipath-tools/libmultipath/discovery.c @@ -129,13 +129,8 @@ path_discovery (vector pathvec, struct c extern int \ sysfs_get_##fname (struct sysfs_device * dev, char * buff, size_t len) \ { \ - char *attr; \ -\ - attr = sysfs_attr_get_value(dev->devpath, #fname); \ - if (!attr) \ + if (sysfs_attr_get_value(dev->devpath, #fname, buff, len) != 0) \ return 1; \ - if (strlcpy(buff, attr, len) != strlen(attr)) \ - return 2; \ strchop(buff); \ return 0; \ } @@ -150,24 +145,17 @@ declare_sysfs_get_str(state); int sysfs_get_dev (struct sysfs_device * dev, char * buff, size_t len) { - char *attr; - - attr = sysfs_attr_get_value(dev->devpath, "dev"); - if (!attr) { + if (sysfs_attr_get_value(dev->devpath, "dev", buff, len) != 0) { condlog(3, "%s: no 'dev' attribute in sysfs", dev->kernel); return 1; } - if (strlcpy(buff, attr, len) != strlen(attr)) { - condlog(3, "%s: overflow in 'dev' attribute", dev->kernel); - return 2; - } return 0; } int sysfs_get_timeout(struct sysfs_device *dev, unsigned int *timeout) { - char *attr; + char buff[NAME_SIZE]; char attr_path[SYSFS_PATH_SIZE]; int r; unsigned int t; @@ -175,11 +163,10 @@ sysfs_get_timeout(struct sysfs_device *d if (safe_sprintf(attr_path, "%s/device", dev->devpath)) return 1; - attr = sysfs_attr_get_value(attr_path, "timeout"); - if (!attr) + if (sysfs_attr_get_value(attr_path, "timeout", buff, NAME_SIZE) != 0) return 1; - r = sscanf(attr, "%u\n", &t); + r = sscanf(buff, "%u\n", &t); if (r != 1) return 1; @@ -192,14 +179,13 @@ sysfs_get_timeout(struct sysfs_device *d int sysfs_get_size (struct sysfs_device * dev, unsigned long long * size) { - char *attr; + char buff[NAME_SIZE]; int r; - attr = sysfs_attr_get_value(dev->devpath, "size"); - if (!attr) + if (sysfs_attr_get_value(dev->devpath, "size", buff, NAME_SIZE) != 0) return 1; - r = sscanf(attr, "%llu\n", size); + r = sscanf(buff, "%llu\n", size); if (r != 1) return 1; @@ -213,7 +199,7 @@ sysfs_get_fc_nodename (struct sysfs_devi unsigned int target) { unsigned int checkhost, session; - char attr_path[SYSFS_PATH_SIZE], *attr; + char attr_path[SYSFS_PATH_SIZE]; if (safe_sprintf(attr_path, "/class/fc_transport/target%i:%i:%i", @@ -222,11 +208,8 @@ sysfs_get_fc_nodename (struct sysfs_devi return 1; } - attr = sysfs_attr_get_value(attr_path, "node_name"); - if (attr) { - strncpy(node, attr, strlen(attr)); + if (!sysfs_attr_get_value(attr_path, "node_name", node, NODE_NAME_SIZE)) return 0; - } if (sscanf(dev->devpath, "/devices/platform/host%u/session%u/", &checkhost, &session) != 2) @@ -238,11 +221,9 @@ sysfs_get_fc_nodename (struct sysfs_devi return 1; } - attr = sysfs_attr_get_value(attr_path, "targetname"); - if (attr) { - strncpy(node, attr, strlen(attr)); + if (!sysfs_attr_get_value(attr_path, "targetname", node, + NODE_NAME_SIZE)) return 0; - } return 1; } @@ -670,14 +651,11 @@ cciss_sysfs_pathinfo (struct path * pp, static int common_sysfs_pathinfo (struct path * pp, struct sysfs_device *dev) { - char *attr; - - attr = sysfs_attr_get_value(dev->devpath, "dev"); - if (!attr) { + if (sysfs_attr_get_value(dev->devpath, "dev", pp->dev_t, + BLK_DEV_SIZE) != 0) { condlog(3, "%s: no 'dev' attribute in sysfs", pp->dev); return 1; } - strlcpy(pp->dev_t, attr, BLK_DEV_SIZE); condlog(3, "%s: dev_t = %s", pp->dev, pp->dev_t); Index: multipath-tools/libmultipath/sysfs.c =================================================================== --- multipath-tools.orig/libmultipath/sysfs.c +++ multipath-tools/libmultipath/sysfs.c @@ -37,15 +37,6 @@ char sysfs_path[PATH_SIZE]; -/* attribute value cache */ -static LIST_HEAD(attr_list); -struct sysfs_attr { - struct list_head node; - char path[PATH_SIZE]; - char *value; /* points to value_local if value is cached */ - char value_local[NAME_SIZE]; -}; - /* list of sysfs devices */ static LIST_HEAD(sysfs_dev_list); struct sysfs_dev { @@ -62,24 +53,15 @@ int sysfs_init(char *path, size_t len) strlcpy(sysfs_path, "/sys", sizeof(sysfs_path)); dbg("sysfs_path='%s'", sysfs_path); - INIT_LIST_HEAD(&attr_list); INIT_LIST_HEAD(&sysfs_dev_list); return 0; } void sysfs_cleanup(void) { - struct sysfs_attr *attr_loop; - struct sysfs_attr *attr_temp; - struct sysfs_dev *sysdev_loop; struct sysfs_dev *sysdev_temp; - list_for_each_entry_safe(attr_loop, attr_temp, &attr_list, node) { - list_del(&attr_loop->node); - free(attr_loop); - } - list_for_each_entry_safe(sysdev_loop, sysdev_temp, &sysfs_dev_list, node) { list_del(&sysdev_loop->node); free(sysdev_loop); @@ -343,6 +325,8 @@ void sysfs_device_put(struct sysfs_devic list_for_each_entry(sysdev_loop, &sysfs_dev_list, node) { if (&sysdev_loop->dev == dev) { + if (dev->parent) + sysfs_device_put(dev->parent); dbg("removed dev '%s' from cache", sysdev_loop->dev.devpath); list_del(&sysdev_loop->node); @@ -350,8 +334,7 @@ void sysfs_device_put(struct sysfs_devic return; } } - dbg("dev '%s' not found in cache", - sysdev_loop->dev.devpath); + dbg("dev '%s' not found in cache", dev->devpath); return; } @@ -416,17 +399,24 @@ out: } -char *sysfs_attr_get_value(const char *devpath, const char *attr_name) +int sysfs_attr_get_value(const char *devpath, const char *attr_name, + char *buff, int len) { char path_full[PATH_SIZE]; const char *path; char value[NAME_SIZE]; - struct sysfs_attr *attr_loop; - struct sysfs_attr *attr = NULL; struct stat statbuf; int fd; ssize_t size; size_t sysfs_len; + int ret = -1; + + if (buff == NULL) { + condlog(0, "no space to store sysfs attr value '%s'", + attr_name); + goto out; + } + memset(buff, 0, len); dbg("open '%s'/'%s'", devpath, attr_name); sysfs_len = strlcpy(path_full, sysfs_path, sizeof(path_full)); @@ -437,29 +427,6 @@ char *sysfs_attr_get_value(const char *d strlcat(path_full, "/", sizeof(path_full)); strlcat(path_full, attr_name, sizeof(path_full)); - /* look for attribute in cache */ - list_for_each_entry(attr_loop, &attr_list, node) { - if (strcmp(attr_loop->path, path) == 0) { - dbg("found in cache '%s'", attr_loop->path); - attr = attr_loop; - } - } - if (!attr) { - /* store attribute in cache */ - dbg("new uncached attribute '%s'", path_full); - attr = malloc(sizeof(struct sysfs_attr)); - if (attr == NULL) - return NULL; - memset(attr, 0x00, sizeof(struct sysfs_attr)); - strlcpy(attr->path, path, sizeof(attr->path)); - dbg("add to cache '%s'", path_full); - list_add(&attr->node, &attr_list); - } else { - /* clear old value */ - if(attr->value) - memset(attr->value, 0x00, sizeof(attr->value)); - } - if (lstat(path_full, &statbuf) != 0) { dbg("stat '%s' failed: %s", path_full, strerror(errno)); goto out; @@ -467,20 +434,27 @@ char *sysfs_attr_get_value(const char *d if (S_ISLNK(statbuf.st_mode)) { /* links return the last element of the target path */ - char link_target[PATH_SIZE]; - int len; + int link_len; const char *pos; - len = readlink(path_full, link_target, sizeof(link_target)); - if (len > 0) { - link_target[len] = '\0'; - pos = strrchr(link_target, '/'); + link_len = readlink(path_full, value, sizeof(value)); + if (link_len > 0) { + if (link_len >= sizeof(value)) { + condlog(0, "overflow in attribute '%s'", + path_full); + goto out; + } + value[link_len] = '\0'; + pos = strrchr(value, '/'); if (pos != NULL) { - dbg("cache '%s' with link value '%s'", - path_full, value); - strlcpy(attr->value_local, &pos[1], - sizeof(attr->value_local)); - attr->value = attr->value_local; + pos++; + if (strlen(pos) >= len) { + condlog(0, "overflow in attribute '%s'", + path_full); + goto out; + } + strncpy(buff, pos, len - 1); + ret = 0; } } goto out; @@ -498,9 +472,9 @@ char *sysfs_attr_get_value(const char *d fd = open(path_full, O_RDONLY); if (fd < 0) { if (errno == EMFILE) - dbg("out of file descriptors. set or increase max_fds in /etc/multipath.conf"); + condlog(0, "out of file descriptors. set or increase max_fds in /etc/multipath.conf"); else - dbg("attribute '%s' can not be opened: %s", + condlog(0, "attribute '%s' can not be opened: %s", path_full, strerror(errno)); goto out; } @@ -512,16 +486,18 @@ char *sysfs_attr_get_value(const char *d dbg("overflow in attribute '%s', truncating", path_full); size--; } - - /* got a valid value, store and return it */ value[size] = '\0'; remove_trailing_chars(value, '\n'); - dbg("cache '%s' with attribute value '%s'", path_full, value); - strlcpy(attr->value_local, value, sizeof(attr->value_local)); - attr->value = attr->value_local; - + strchop(value); + if (strlen(value) >= len) { + condlog(0, "overflow in attribute '%s'", path_full); + goto out; + } + strncpy(buff, value, len - 1); + /* got a valid value, store and return it */ + ret = 0; out: - return attr && attr->value && strlen(attr->value) ? attr->value : NULL; + return ret; } int sysfs_lookup_devpath_by_subsys_id(char *devpath_full, size_t len, Index: multipath-tools/libmultipath/sysfs.h =================================================================== --- multipath-tools.orig/libmultipath/sysfs.h +++ multipath-tools/libmultipath/sysfs.h @@ -19,7 +19,8 @@ struct sysfs_device *sysfs_device_get(co struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev); struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem); void sysfs_device_put(struct sysfs_device *dev); -char *sysfs_attr_get_value(const char *devpath, const char *attr_name); +int sysfs_attr_get_value(const char *devpath, const char *attr_name, + char *buff, int len); int sysfs_resolve_link(char *path, size_t size); int sysfs_get_size (struct sysfs_device * dev, unsigned long long * size); int sysfs_attr_set_value(const char *devpath, const char *attr_name, Index: multipath-tools/multipathd/main.c =================================================================== --- multipath-tools.orig/multipathd/main.c +++ multipath-tools/multipathd/main.c @@ -228,16 +228,17 @@ int ev_add_map (struct sysfs_device * dev, struct vectors * vecs) { char * alias; - char *dev_t; + char dev_t[BLK_DEV_SIZE]; int major, minor; char * refwwid; struct multipath * mpp; int map_present; int r = 1; - dev_t = sysfs_attr_get_value(dev->devpath, "dev"); + if (sysfs_attr_get_value(dev->devpath, "dev", dev_t, BLK_DEV_SIZE) != 0) + return 1; - if (!dev_t || sscanf(dev_t, "%d:%d", &major, &minor) != 2) + if (sscanf(dev_t, "%d:%d", &major, &minor) != 2) return 1; alias = dm_mapname(major, minor);