path: root/0081-RHBZ-623644-fix-sysfs-caching.patch

---
 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);