Skip to contentSkip to conversation

staging: greybus: loopback_test: fix build breakage

PATCH New Johan Hovold 1 participant 4 messages Mar 12, 2020

Subsystems (3)
lore
Share & tools
lore

Johan Hovold

author maintainer
#0 lore raw

cover staging: greybus: loopback_test: fix build breakage

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(-)
Signature · 1 line
2.24.1
author maintainer
#1 lore raw

PATCH 1/3 staging: greybus: loopback_test: fix poll-mask build breakage

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 <[email protected]> # 4.16
---
 drivers/staging/greybus/tools/loopback_test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/greybus/tools/loopback_test.c +2 -2
index ba6f905f26fa..41e1820d9ac9 100644
@@ -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++;
Signature · 1 line
2.24.1
author maintainer
#2 lore raw

PATCH 2/3 staging: greybus: loopback_test: fix potential path truncation

In reply to Johan Hovold

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")
---
 drivers/staging/greybus/tools/loopback_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/greybus/tools/loopback_test.c +1 -1
index 41e1820d9ac9..d38bb4fbd6b9 100644
@@ -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;
Signature · 1 line
2.24.1
author maintainer
#3 lore raw

PATCH 3/3 staging: greybus: loopback_test: fix potential path truncations

In reply to Johan Hovold

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")
---
 drivers/staging/greybus/tools/loopback_test.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/staging/greybus/tools/loopback_test.c +8 -7
index d38bb4fbd6b9..69c6dce9be31 100644
@@ -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)
Signature · 1 line
2.24.1