From: Martin Wilck <[email protected]> snprintf() returns int, but the size argument "n" is size_t. Use safe_snprintf() to avoid -Wsign-compare warnings. At the same time, improve these macros to check for errors in snprintf(), too.
Note: there are more uses of snprintf() in our code that may need review, too. For now, I'm fixing only those that were causing warnings. Signed-off-by: Martin Wilck <[email protected]> --- kpartx/devmapper.c | 3 ++- kpartx/kpartx.h | 11 ++++++++++- libmultipath/foreign/nvme.c | 6 ++---- libmultipath/sysfs.c | 7 +++---- libmultipath/util.c | 3 +-- libmultipath/util.h | 11 +++++++++-- libmultipath/wwids.c | 3 +-- multipath/main.c | 3 +-- 8 files changed, 29 insertions(+), 18 deletions(-) diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c index 3aa4988d..b100b5ef 100644 --- a/kpartx/devmapper.c +++ b/kpartx/devmapper.c @@ -10,6 +10,7 @@ #include <errno.h> #include <sys/sysmacros.h> #include "devmapper.h" +#include "kpartx.h" #define _UUID_PREFIX "part" #define UUID_PREFIX _UUID_PREFIX "%d-" @@ -107,7 +108,7 @@ strip_slash (char * device) static int format_partname(char *buf, size_t bufsiz, const char *mapname, const char *delim, int part) { - if (snprintf(buf, bufsiz, "%s%s%d", mapname, delim, part) >= bufsiz) + if (safe_snprintf(buf, bufsiz, "%s%s%d", mapname, delim, part)) return 0; strip_slash(buf); return 1; diff --git a/kpartx/kpartx.h b/kpartx/kpartx.h index 52920e43..2d36b0b5 100644 --- a/kpartx/kpartx.h +++ b/kpartx/kpartx.h @@ -16,8 +16,17 @@ #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0) +#define safe_snprintf(var, size, format, args...) \ + ({ \ + size_t __size = size; \ + int __ret; \ + \ + __ret = snprintf(var, size, format, ##args); \ + __ret < 0 || (size_t)__ret >= __size; \ + }) + #define safe_sprintf(var, format, args...) \ - snprintf(var, sizeof(var), format, ##args) >= sizeof(var) + safe_snprintf(var, sizeof(var), format, ##args) #ifndef BLKSSZGET #define BLKSSZGET _IO(0x12,104) /* get block device sector size */ diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c index e8ca516c..09cdddf0 100644 --- a/libmultipath/foreign/nvme.c +++ b/libmultipath/foreign/nvme.c @@ -591,8 +591,7 @@ static void test_ana_support(struct nvme_map *map, struct udev_device *ctl) return; dev_t = udev_device_get_sysattr_value(ctl, "dev"); - if (snprintf(sys_path, sizeof(sys_path), "/dev/char/%s", dev_t) - >= sizeof(sys_path)) + if (safe_sprintf(sys_path, "/dev/char/%s", dev_t)) return; fd = open(sys_path, O_RDONLY); @@ -663,8 +662,7 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map) char *fn = di[i]->d_name; struct udev_device *ctrl, *udev; - if (snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s", fn) - >= sizeof(pathbuf) - n) + if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s", fn)) continue; if (realpath(pathbuf, realbuf) == NULL) { condlog(3, "%s: %s: realpath: %s", __func__, THIS, diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c index 923b529b..62ec2ed7 100644 --- a/libmultipath/sysfs.c +++ b/libmultipath/sysfs.c @@ -306,7 +306,7 @@ bool sysfs_is_multipathed(const struct path *pp) n = snprintf(pathbuf, sizeof(pathbuf), "/sys/block/%s/holders", pp->dev); - if (n >= sizeof(pathbuf)) { + if (n < 0 || (size_t)n >= sizeof(pathbuf)) { condlog(1, "%s: pathname overflow", __func__); return false; } @@ -327,9 +327,8 @@ bool sysfs_is_multipathed(const struct path *pp) int nr; char uuid[6]; - if (snprintf(pathbuf + n, sizeof(pathbuf) - n, - "/%s/dm/uuid", di[i]->d_name) - >= sizeof(pathbuf) - n) + if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n, + "/%s/dm/uuid", di[i]->d_name)) continue; fd = open(pathbuf, O_RDONLY); diff --git a/libmultipath/util.c b/libmultipath/util.c index ccc0de29..51c38c87 100644 --- a/libmultipath/util.c +++ b/libmultipath/util.c @@ -212,8 +212,7 @@ int devt2devname(char *devname, int devname_len, char *devt) continue; if ((major == tmpmaj) && (minor == tmpmin)) { - if (snprintf(block_path, sizeof(block_path), - "/sys/block/%s", dev) >= sizeof(block_path)) { + if (safe_sprintf(block_path, "/sys/block/%s", dev)) { condlog(0, "device name %s is too long", dev); fclose(fd); return 1; diff --git a/libmultipath/util.h b/libmultipath/util.h index 913ab7c2..12ce723f 100644 --- a/libmultipath/util.h +++ b/libmultipath/util.h @@ -29,9 +29,16 @@ void set_max_fds(rlim_t max_fds); #define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0])) #define safe_sprintf(var, format, args...) \ - snprintf(var, sizeof(var), format, ##args) >= sizeof(var) + safe_snprintf(var, sizeof(var), format, ##args) + #define safe_snprintf(var, size, format, args...) \ - snprintf(var, size, format, ##args) >= size + ({ \ + size_t __size = size; \ + int __ret; \ + \ + __ret = snprintf(var, size, format, ##args); \ + __ret < 0 || (size_t)__ret >= __size; \ + }) #define pthread_cleanup_push_cast(f, arg) \ pthread_cleanup_push(((void (*)(void *))&f), (arg)) diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c index 291db8f5..28a2150d 100644 --- a/libmultipath/wwids.c +++ b/libmultipath/wwids.c @@ -393,8 +393,7 @@ static int _failed_wwid_op(const char *wwid, bool rw, long lockfd; int r = -1; - if (snprintf(path, sizeof(path), "%s/%s", shm_dir, wwid) - >= sizeof(path)) { + if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) { condlog(1, "%s: path name overflow", __func__); return -1; } diff --git a/multipath/main.c b/multipath/main.c index c2ef8c7b..c553437b 100644 --- a/multipath/main.c +++ b/multipath/main.c @@ -423,8 +423,7 @@ static int find_multipaths_check_timeout(const struct path *pp, long tmo, clock_gettime(CLOCK_REALTIME, &now); - if (snprintf(path, sizeof(path), "%s/%s", shm_find_mp_dir, pp->dev_t) - >= sizeof(path)) { + if (safe_sprintf(path, "%s/%s", shm_find_mp_dir, pp->dev_t)) { condlog(1, "%s: path name overflow", __func__); return FIND_MULTIPATHS_ERROR; } -- 2.23.0 -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
