[dm-devel] [PATCH 07/14] libmultipath: sysfs_attr_get_value(): don't return 0 if buffer too small

[mwilck@suse.com]

From: Martin Wilck <mwilck@suse.com>

If the passed read buffer is too small to hold the value read plus
terminating 0 byte, return the given size value rather than 0.

This way we get similar semantics as for sysfs_bin_attr_get_get_value(),
except that sysfs_attr_get_value() must always 0-terminate the value;
thus a return value equal to the length parameter is an error for
the non-binary case.

Provide a helper macro to test this "overflow" condition.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c |  2 +-
 libmultipath/discovery.c | 14 +++++++-------
 libmultipath/propsel.c   |  6 +++++-
 libmultipath/sysfs.c     |  3 +--
 libmultipath/sysfs.h     | 13 +++++++++++++
 multipathd/main.c        |  2 +-
 6 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 09ae708..467bbaa 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -589,7 +589,7 @@ sysfs_set_max_sectors_kb(struct multipath *mpp, int is_reload)
 		ret = sysfs_attr_get_value(udd, "queue/max_sectors_kb", buff,
 					   sizeof(buff));
 		udev_device_unref(udd);
-		if (ret <= 0) {
+		if (!sysfs_attr_value_ok(ret, sizeof(buff))) {
 			condlog(1, "failed to get current max_sectors_kb from %s", mpp->alias);
 			return 1;
 		}
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index f5b8401..54b1caf 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -560,10 +560,10 @@ sysfs_get_asymmetric_access_state(struct path *pp, char *buff, int buflen)
 	if (!parent)
 		return -1;
 
-	if (sysfs_attr_get_value(parent, "access_state", buff, buflen) <= 0)
+	if (!sysfs_attr_get_value_ok(parent, "access_state", buff, buflen))
 		return -1;
 
-	if (sysfs_attr_get_value(parent, "preferred_path", value, 16) <= 0)
+	if (!sysfs_attr_get_value_ok(parent, "preferred_path", value, sizeof(value)))
 		return 0;
 
 	preferred = strtoul(value, &eptr, 0);
@@ -638,8 +638,8 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	/*
 	 * read the current dev_loss_tmo value from sysfs
 	 */
-	ret = sysfs_attr_get_value(rport_dev, "dev_loss_tmo", value, 16);
-	if (ret <= 0) {
+	ret = sysfs_attr_get_value(rport_dev, "dev_loss_tmo", value, sizeof(value));
+	if (!sysfs_attr_value_ok(ret, sizeof(value))) {
 		condlog(0, "%s: failed to read dev_loss_tmo value, "
 			"error %d", rport_id, -ret);
 		goto out;
@@ -1737,8 +1737,8 @@ path_offline (struct path * pp)
 	}
 
 	memset(buff, 0x0, SCSI_STATE_SIZE);
-	err = sysfs_attr_get_value(parent, "state", buff, SCSI_STATE_SIZE);
-	if (err <= 0) {
+	err = sysfs_attr_get_value(parent, "state", buff, sizeof(buff));
+	if (!sysfs_attr_value_ok(err, sizeof(buff))) {
 		if (err == -ENXIO)
 			return PATH_REMOVED;
 		else
@@ -2142,7 +2142,7 @@ static ssize_t uid_fallback(struct path *pp, int path_state,
 			return -1;
 		len = sysfs_attr_get_value(pp->udev, "wwid", value,
 					   sizeof(value));
-		if (len <= 0)
+		if (!sysfs_attr_value_ok(len, sizeof(value)))
 			return -1;
 		len = strlcpy(pp->wwid, value, WWID_SIZE);
 		if (len >= WWID_SIZE) {
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 50d0b5c..18f2277 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -435,6 +435,7 @@ out:
 static int get_dh_state(struct path *pp, char *value, size_t value_len)
 {
 	struct udev_device *ud;
+	ssize_t rc;
 
 	if (pp->udev == NULL)
 		return -1;
@@ -444,7 +445,10 @@ static int get_dh_state(struct path *pp, char *value, size_t value_len)
 	if (ud == NULL)
 		return -1;
 
-	return sysfs_attr_get_value(ud, "dh_state", value, value_len);
+	rc = sysfs_attr_get_value(ud, "dh_state", value, value_len);
+	if (!sysfs_attr_value_ok(rc, value_len))
+		return -1;
+	return rc;
 }
 
 int select_hwhandler(struct config *conf, struct multipath *mp)
diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index e48b05e..125f1c2 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -85,7 +85,6 @@ static ssize_t __sysfs_attr_get_value(struct udev_device *dev, const char *attr_
 		condlog(3, "%s: overflow reading from %s (required len: %zu)",
 			__func__, devpath, size);
 		value[size - 1] = '\0';
-		size = 0;
 	} else if (!binary) {
 		value[size] = '\0';
 		size = strchop(value);
@@ -165,7 +164,7 @@ sysfs_get_size (struct path *pp, unsigned long long * size)
 		return 1;
 
 	attr[0] = '\0';
-	if (sysfs_attr_get_value(pp->udev, "size", attr, 255) <= 0) {
+	if (!sysfs_attr_get_value_ok(pp->udev, "size", attr, sizeof(attr))) {
 		condlog(3, "%s: No size attribute in sysfs", pp->dev);
 		return 1;
 	}
diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
index 72b39ab..cdc84e4 100644
--- a/libmultipath/sysfs.h
+++ b/libmultipath/sysfs.h
@@ -12,6 +12,19 @@ ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
 			     char * value, size_t value_len);
 ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
 				 unsigned char * value, size_t value_len);
+#define sysfs_attr_value_ok(rc, value_len)			\
+	({							\
+		ssize_t __r = rc;				\
+		__r >= 0 && (size_t)__r < (size_t)value_len;	\
+	})
+
+#define sysfs_attr_get_value_ok(dev, attr, val, len) \
+	({ \
+		size_t __l = (len);					\
+		ssize_t __rc = sysfs_attr_get_value(dev, attr, val, __l); \
+		sysfs_attr_value_ok(__rc, __l); \
+	})
+
 int sysfs_get_size (struct path *pp, unsigned long long * size);
 int sysfs_check_holders(char * check_devt, char * new_devt);
 bool sysfs_is_multipathed(struct path *pp, bool set_wwid);
diff --git a/multipathd/main.c b/multipathd/main.c
index 2f2b9d4..68eca92 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1126,7 +1126,7 @@ sysfs_get_ro (struct path *pp)
 	if (!pp->udev)
 		return -1;
 
-	if (sysfs_attr_get_value(pp->udev, "ro", buff, sizeof(buff)) <= 0) {
+	if (!sysfs_attr_get_value_ok(pp->udev, "ro", buff, sizeof(buff))) {
 		condlog(3, "%s: Cannot read ro attribute in sysfs", pp->dev);
 		return -1;
 	}
-- 
2.36.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel