[RFC PATCH] pasta, passt-repair: Support multiple events per read() in inotify handlers

[Stefano Brivio <sbrivio@redhat.com>]

The current code assumes that we'll get one event per read() on
inotify descriptors, but that's not the case, not from documentation,
and not from reports.

Add loops in the two inotify handlers we have, in pasta-specific code
and passt-repair, to go through all the events we receive.

While in pasta the the inotify watch is in an epoll list, and we'll
get epoll wakeups as long as there's data to read, in passt-repair
that's not the case. So, in pasta we can simply size the buffer for
a single event and try to read one, but in passt-repair, we'll need
to size the buffer to a safer, reasonable amount of events.

Link: https://bugs.passt.top/show_bug.cgi?id=119
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
I'm posting this as RFC because, while it seems to do the job and I
tested all the code paths, Coverity isn't amused by the fact that
we assume that inotify 'name' attributes (ev->name) are
NULL-terminated. They actually are, but the code is not very robust.

Addressing that is kind of trivial but I keep getting it wrong, so
I'll start posting this and fix that up later (direct fixes/edits of
this patch are also welcome, of course).

 passt-repair.c | 26 +++++++++++++++++++-------
 pasta.c        | 20 +++++++++++++-------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/passt-repair.c b/passt-repair.c
index 120f7aa..86581c6 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -111,14 +111,14 @@ int main(int argc, char **argv)
 	}
 
 	if ((sb.st_mode & S_IFMT) == S_IFDIR) {
-		char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
+		char buf[(sizeof(struct inotify_event) + NAME_MAX + 1) * 128]
+		   __attribute__ ((aligned(__alignof__(struct inotify_event))));
 		const struct inotify_event *ev;
 		char path[PATH_MAX + 1];
+		bool found = false;
 		ssize_t n;
 		int fd;
 
-		ev = (struct inotify_event *)buf;
-
 		if ((fd = inotify_init1(IN_CLOEXEC)) < 0) {
 			fprintf(stderr, "inotify_init1: %i\n", errno);
 			_exit(1);
@@ -130,6 +130,8 @@ int main(int argc, char **argv)
 		}
 
 		do {
+			char *p;
+
 			n = read(fd, buf, sizeof(buf));
 			if (n < 0) {
 				fprintf(stderr, "inotify read: %i", errno);
@@ -138,11 +140,21 @@ int main(int argc, char **argv)
 
 			if (n < (ssize_t)sizeof(*ev)) {
 				fprintf(stderr, "Short inotify read: %zi", n);
-				_exit(1);
+				continue;
+			}
+
+			for (p = buf; p < buf + n; p += sizeof(*ev) + ev->len) {
+				ev = (const struct inotify_event *)p;
+
+				if (ev->len >= REPAIR_EXT_LEN &&
+				    !memcmp(ev->name + strlen(ev->name) -
+					    REPAIR_EXT_LEN,
+					    REPAIR_EXT, REPAIR_EXT_LEN)) {
+					found = true;
+					break;
+				}
 			}
-		} while (ev->len < REPAIR_EXT_LEN ||
-			 memcmp(ev->name + strlen(ev->name) - REPAIR_EXT_LEN,
-				REPAIR_EXT, REPAIR_EXT_LEN));
+		} while (!found);
 
 		snprintf(path, sizeof(path), "%s/%s", argv[1], ev->name);
 		if ((stat(path, &sb))) {
diff --git a/pasta.c b/pasta.c
index fa3e7de..017fa32 100644
--- a/pasta.c
+++ b/pasta.c
@@ -498,17 +498,23 @@ void pasta_netns_quit_init(const struct ctx *c)
  */
 void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)
 {
-	char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
-	const struct inotify_event *in_ev = (struct inotify_event *)buf;
+	char buf[sizeof(struct inotify_event) + NAME_MAX + 1]
+		__attribute__ ((aligned(__alignof__(struct inotify_event))));
+	const struct inotify_event *ev;
+	ssize_t n;
+	char *p;
 
-	if (read(inotify_fd, buf, sizeof(buf)) < (ssize_t)sizeof(*in_ev))
+	if ((n = read(inotify_fd, buf, sizeof(buf))) < (ssize_t)sizeof(*ev))
 		return;
 
-	if (strncmp(in_ev->name, c->netns_base, sizeof(c->netns_base)))
-		return;
+	for (p = buf; p < buf + n; p += sizeof(*ev) + ev->len) {
+		ev = (const struct inotify_event *)p;
 
-	info("Namespace %s is gone, exiting", c->netns_base);
-	_exit(EXIT_SUCCESS);
+		if (!strncmp(ev->name, c->netns_base, sizeof(c->netns_base))) {
+			info("Namespace %s is gone, exiting", c->netns_base);
+			_exit(EXIT_SUCCESS);
+		}
+	}
 }
 
 /**
-- 
@@ -498,17 +498,23 @@ void pasta_netns_quit_init(const struct ctx *c)
  */
 void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)
 {
-	char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
-	const struct inotify_event *in_ev = (struct inotify_event *)buf;
+	char buf[sizeof(struct inotify_event) + NAME_MAX + 1]
+		__attribute__ ((aligned(__alignof__(struct inotify_event))));
+	const struct inotify_event *ev;
+	ssize_t n;
+	char *p;
 
-	if (read(inotify_fd, buf, sizeof(buf)) < (ssize_t)sizeof(*in_ev))
+	if ((n = read(inotify_fd, buf, sizeof(buf))) < (ssize_t)sizeof(*ev))
 		return;
 
-	if (strncmp(in_ev->name, c->netns_base, sizeof(c->netns_base)))
-		return;
+	for (p = buf; p < buf + n; p += sizeof(*ev) + ev->len) {
+		ev = (const struct inotify_event *)p;
 
-	info("Namespace %s is gone, exiting", c->netns_base);
-	_exit(EXIT_SUCCESS);
+		if (!strncmp(ev->name, c->netns_base, sizeof(c->netns_base))) {
+			info("Namespace %s is gone, exiting", c->netns_base);
+			_exit(EXIT_SUCCESS);
+		}
+	}
 }
 
 /**
-- 
2.43.0