From a0343f326891ab30ca917f3c992b7b8fc543a92d Mon Sep 17 00:00:00 2001 From: Robin Getz Date: Fri, 24 Apr 2020 11:43:45 -0400 Subject: [PATCH] xml buildups: fix bug introduced by recent changes be more pedanic about the differences between strlen (doesn't include null char) and sizeof (which does include null char). Now, we calculate the exact length the the string (number of char) add one for malloc, fill everything out, and then check it make sure the length is one (the null char) left. tested on host and local (Zedboard + FMCOMMS2) Signed-off-by: Robin Getz --- channel.c | 29 +++++++++++++++++++++-------- context.c | 3 ++- device.c | 13 +++++++++---- iio-private.h | 33 ++++++++++++++++++++++++--------- 4 files changed, 56 insertions(+), 22 deletions(-) diff --git a/channel.c b/channel.c index 10401d595..9ff3b012e 100644 --- a/channel.c +++ b/channel.c @@ -179,20 +179,28 @@ void iio_channel_init_finalize(struct iio_channel *chn) static char *get_attr_xml(struct iio_channel_attr *attr, size_t *length) { char *str; - size_t len = strlen(attr->name) + sizeof(""); - if (attr->filename) - len += strlen(attr->filename) + sizeof("filename=\"\""); + size_t len; + len = strnlen(attr->name, MAX_ATTR_NAME); + len += sizeof("") - 1; + + if (attr->filename) { + len += strnlen(attr->filename, NAME_MAX); + len += sizeof(" filename=\"\"") - 1; + } + + *length = len; /* just the chars */ + len++; /* room for terminating NULL */ str = malloc(len); if (!str) return NULL; - *length = len - 1; /* Skip the \0 */ if (attr->filename) iio_snprintf(str, len, "", attr->name, attr->filename); else iio_snprintf(str, len, "", attr->name); + return str; } @@ -231,10 +239,13 @@ char * iio_channel_get_xml(const struct iio_channel *chn, size_t *length) size_t *attrs_len, scan_element_len = 0; unsigned int i; - len = sizeof(""); + len = sizeof("") - 1; len += strnlen(chn->id, MAX_CHN_ID); - len += chn->is_output ? sizeof("output") : sizeof("input"); - len += chn->name ? sizeof(" name= ") + strnlen(chn->name, MAX_CHN_NAME) : 0; + len += (chn->is_output ? sizeof("output") : sizeof("input")) - 1; + if (chn->name) { + len += sizeof(" name=\"\"") - 1; + len += strnlen(chn->name, MAX_CHN_NAME); + } if (chn->is_scan_element) { scan_element = get_scan_element(chn, &scan_element_len); @@ -260,6 +271,7 @@ char * iio_channel_get_xml(const struct iio_channel *chn, size_t *length) len += attrs_len[i]; } + len++; /* room for terminating NULL */ str = malloc(len); if (!str) goto err_free_attrs; @@ -307,7 +319,8 @@ char * iio_channel_get_xml(const struct iio_channel *chn, size_t *length) *length = ptr - str; - if (len < 0) { + /* NULL char should be left, and that is it */ + if (len != 1) { IIO_ERROR("Internal libIIO error: iio_channel_get_xml str length isssue\n"); free(str); return NULL; diff --git a/context.c b/context.c index aba2c9381..43373c78f 100644 --- a/context.c +++ b/context.c @@ -89,6 +89,7 @@ char * iio_context_create_xml(const struct iio_context *ctx) } } + len++; /* room for terminating NULL */ str = malloc(len); if (!str) { errno = ENOMEM; @@ -132,7 +133,7 @@ char * iio_context_create_xml(const struct iio_context *ctx) len -= sizeof("") - 1; } - if (len < 0) { + if (len != 1) { IIO_ERROR("Internal libIIO error: iio_context_create_xml str length isssue\n"); free(str); return NULL; diff --git a/device.c b/device.c index e847264a0..5ffb86ee1 100644 --- a/device.c +++ b/device.c @@ -26,9 +26,12 @@ static char *get_attr_xml(const char *attr, size_t *length, enum iio_attr_type type) { - size_t len = sizeof("") + strlen(attr); + size_t len; char *str; + len = sizeof("") - 1; + len += strnlen(attr, MAX_ATTR_NAME); + switch(type){ case IIO_ATTR_TYPE_DEVICE: break; @@ -42,11 +45,12 @@ static char *get_attr_xml(const char *attr, size_t *length, enum iio_attr_type t return NULL; } + *length = len; /* just the chars */ + len++; /* room for terminating NULL */ str = malloc(len); if (!str) return NULL; - *length = len - 1; /* Skip the \0 */ switch (type) { case IIO_ATTR_TYPE_DEVICE: iio_snprintf(str, len, "", attr); @@ -70,7 +74,7 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length) size_t *attrs_len, *channels_len, *buffer_attrs_len, *debug_attrs_len; unsigned int i, j, k; - len = sizeof(" ") - 1; + len = sizeof("") - 1; len += strnlen(dev->id, MAX_DEV_ID); if (dev->name) { len += sizeof(" name=\"\"") - 1; @@ -146,6 +150,7 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length) len += debug_attrs_len[k]; } + len++; /* room for terminating NULL */ str = malloc(len); if (!str) goto err_free_debug_attrs; @@ -222,7 +227,7 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length) *length = ptr - str; - if (len < 0) { + if (len != 1) { IIO_ERROR("Internal libIIO error: iio_device_get_xml str length isssue\n"); free(str); return NULL; diff --git a/iio-private.h b/iio-private.h index dafcfd841..4e82b3d22 100644 --- a/iio-private.h +++ b/iio-private.h @@ -60,15 +60,30 @@ #define CLEAR_BIT(addr, bit) \ *(((uint32_t *) addr) + BIT_WORD(bit)) &= ~BIT_MASK(bit) -/* 256 is the MAX_NAME (file name) on Linux, 4096 is PAGESIZE */ -#define MAX_CHN_ID 256 /* encoded in the sysfs filename */ -#define MAX_CHN_NAME 256 /* encoded in the sysfs filename */ -#define MAX_DEV_ID 256 /* encoded in the sysfs filename */ -#define MAX_DEV_NAME 256 /* encoded in the sysfs filename */ -#define MAX_CTX_NAME 256 /* nominally "xml" */ -#define MAX_CTX_DESC 256 /* nominally "linux ..." */ -#define MAX_ATTR_NAME 256 /* encoded in the sysfs filename */ -#define MAX_ATTR_VALUE 4096 /* Linux page size, could be anything */ +/* https://pubs.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html + * {NAME_MAX} : Maximum number of bytes in a filename + * {PATH_MAX} : Maximum number of bytes in a pathname + * {PAGESIZE} : Size in bytes of a page + * Too bad we work on non-POSIX systems + */ +#ifndef NAME_MAX +#define NAME_MAX 256 +#endif +#ifndef PATH_MAX +#define PATH_MAX 4096 +#endif +#ifndef PAGESIZE +#define PAGESIZE 4096 +#endif + +#define MAX_CHN_ID NAME_MAX /* encoded in the sysfs filename */ +#define MAX_CHN_NAME NAME_MAX /* encoded in the sysfs filename */ +#define MAX_DEV_ID NAME_MAX /* encoded in the sysfs filename */ +#define MAX_DEV_NAME NAME_MAX /* encoded in the sysfs filename */ +#define MAX_CTX_NAME NAME_MAX /* nominally "xml" */ +#define MAX_CTX_DESC NAME_MAX /* nominally "linux ..." */ +#define MAX_ATTR_NAME NAME_MAX /* encoded in the sysfs filename */ +#define MAX_ATTR_VALUE PAGESIZE /* Linux page size, could be anything */ /* ntohl/htonl are a nightmare to use in cross-platform applications, * since they are defined in different headers on different platforms.