The loopback_test tool hasn't received much love lately. In fact, it has failed to build for the past two years after a scripted EPOLL* conversion.
Newer GCC also started warning for potential string truncation of generated path names; the last two patches addresses that.
Johan
Johan Hovold (3): staging: greybus: loopback_test: fix poll-mask build breakage staging: greybus: loopback_test: fix potential path truncation staging: greybus: loopback_test: fix potential path truncations
drivers/staging/greybus/tools/loopback_test.c | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-)
A scripted conversion from userland POLL* to kernel EPOLL* constants mistakingly replaced the poll flags in the loopback_test tool, which therefore no longer builds.
Fixes: a9a08845e9ac ("vfs: do bulk POLL* -> EPOLL* replacement") Cc: stable stable@vger.kernel.org # 4.16 Signed-off-by: Johan Hovold johan@kernel.org --- drivers/staging/greybus/tools/loopback_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index ba6f905f26fa..41e1820d9ac9 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -655,7 +655,7 @@ static int open_poll_files(struct loopback_test *t) goto err; } read(t->fds[fds_idx].fd, &dummy, 1); - t->fds[fds_idx].events = EPOLLERR|EPOLLPRI; + t->fds[fds_idx].events = POLLERR | POLLPRI; t->fds[fds_idx].revents = 0; fds_idx++; } @@ -748,7 +748,7 @@ static int wait_for_complete(struct loopback_test *t) }
for (i = 0; i < t->poll_count; i++) { - if (t->fds[i].revents & EPOLLPRI) { + if (t->fds[i].revents & POLLPRI) { /* Dummy read to clear the event */ read(t->fds[i].fd, &dummy, 1); number_of_events++;
Newer GCC warns about a possible truncation of a generated sysfs path name as we're concatenating a directory path with a file name and placing the result in a buffer that is half the size of the maximum length of the directory path (which is user controlled).
loopback_test.c: In function 'open_poll_files': loopback_test.c:651:31: warning: '%s' directive output may be truncated writing up to 511 bytes into a region of size 255 [-Wformat-truncation=] 651 | snprintf(buf, sizeof(buf), "%s%s", dev->sysfs_entry, "iteration_count"); | ^~ loopback_test.c:651:3: note: 'snprintf' output between 16 and 527 bytes into a destination of size 255 651 | snprintf(buf, sizeof(buf), "%s%s", dev->sysfs_entry, "iteration_count"); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix this by making sure the buffer is large enough the concatenated strings.
Fixes: 6b0658f68786 ("greybus: tools: Add tools directory to greybus repo and add loopback") Fixes: 9250c0ee2626 ("greybus: Loopback_test: use poll instead of inotify") Signed-off-by: Johan Hovold johan@kernel.org --- drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 41e1820d9ac9..d38bb4fbd6b9 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -637,7 +637,7 @@ int find_loopback_devices(struct loopback_test *t) static int open_poll_files(struct loopback_test *t) { struct loopback_device *dev; - char buf[MAX_STR_LEN]; + char buf[MAX_SYSFS_PATH + MAX_STR_LEN]; char dummy; int fds_idx = 0; int i;
Newer GCC warns about possible truncations of two generated path names as we're concatenating the configurable sysfs and debugfs path prefixes with a filename and placing the results in buffers of the same size as the maximum length of the prefixes.
snprintf(d->name, MAX_STR_LEN, "gb_loopback%u", dev_id);
snprintf(d->sysfs_entry, MAX_SYSFS_PATH, "%s%s/", t->sysfs_prefix, d->name);
snprintf(d->debugfs_entry, MAX_SYSFS_PATH, "%sraw_latency_%s", t->debugfs_prefix, d->name);
Fix this by separating the maximum path length from the maximum prefix length and reducing the latter enough to fit the generated strings.
Note that we also need to reduce the device-name buffer size as GCC isn't smart enough to figure out that we ever only used MAX_STR_LEN bytes of it.
Fixes: 6b0658f68786 ("greybus: tools: Add tools directory to greybus repo and add loopback") Signed-off-by: Johan Hovold johan@kernel.org --- drivers/staging/greybus/tools/loopback_test.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index d38bb4fbd6b9..69c6dce9be31 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -19,6 +19,7 @@ #include <signal.h>
#define MAX_NUM_DEVICES 10 +#define MAX_SYSFS_PREFIX 0x80 #define MAX_SYSFS_PATH 0x200 #define CSV_MAX_LINE 0x1000 #define SYSFS_MAX_INT 0x20 @@ -67,7 +68,7 @@ struct loopback_results { };
struct loopback_device { - char name[MAX_SYSFS_PATH]; + char name[MAX_STR_LEN]; char sysfs_entry[MAX_SYSFS_PATH]; char debugfs_entry[MAX_SYSFS_PATH]; struct loopback_results results; @@ -93,8 +94,8 @@ struct loopback_test { int stop_all; int poll_count; char test_name[MAX_STR_LEN]; - char sysfs_prefix[MAX_SYSFS_PATH]; - char debugfs_prefix[MAX_SYSFS_PATH]; + char sysfs_prefix[MAX_SYSFS_PREFIX]; + char debugfs_prefix[MAX_SYSFS_PREFIX]; struct timespec poll_timeout; struct loopback_device devices[MAX_NUM_DEVICES]; struct loopback_results aggregate_results; @@ -907,10 +908,10 @@ int main(int argc, char *argv[]) t.iteration_max = atoi(optarg); break; case 'S': - snprintf(t.sysfs_prefix, MAX_SYSFS_PATH, "%s", optarg); + snprintf(t.sysfs_prefix, MAX_SYSFS_PREFIX, "%s", optarg); break; case 'D': - snprintf(t.debugfs_prefix, MAX_SYSFS_PATH, "%s", optarg); + snprintf(t.debugfs_prefix, MAX_SYSFS_PREFIX, "%s", optarg); break; case 'm': t.mask = atol(optarg); @@ -961,10 +962,10 @@ int main(int argc, char *argv[]) }
if (!strcmp(t.sysfs_prefix, "")) - snprintf(t.sysfs_prefix, MAX_SYSFS_PATH, "%s", sysfs_prefix); + snprintf(t.sysfs_prefix, MAX_SYSFS_PREFIX, "%s", sysfs_prefix);
if (!strcmp(t.debugfs_prefix, "")) - snprintf(t.debugfs_prefix, MAX_SYSFS_PATH, "%s", debugfs_prefix); + snprintf(t.debugfs_prefix, MAX_SYSFS_PREFIX, "%s", debugfs_prefix);
ret = find_loopback_devices(&t); if (ret)
-
Johan Hovold