[dm-devel] [PATCH 07/14] libmultipath: sysfs_attr_get_value(): don't return 0 if buffer too small
mwilck at suse.com
mwilck at suse.com
Wed Jul 6 14:38:15 UTC 2022
From: Martin Wilck <mwilck at 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 at 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
More information about the dm-devel
mailing list