From: Martin Wilck <mwilck@xxxxxxxx> 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@xxxxxxxx> --- 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@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel