On Fri, 28 Mar 2025 11:39:58 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:
From: Stefano Brivio <sbrivio(a)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.
Link:
https://bugs.passt.top/show_bug.cgi?id=119
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
[dwg: Remove unnecessary buffer expansion, use strnlen instead of strlen
to make Coverity happier]
I have to admit I didn't know about strnlen(). POSIX.1-2008, that's
bleeding edge stuff. :)
That wasn't actually enough though: in my Coverity scan, it was also
complaining about:
Signed-off-by: David Gibson
<david(a)gibson.dropbear.id.au>
---
passt-repair.c | 27 ++++++++++++++++++++-------
pasta.c | 20 +++++++++++++-------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/passt-repair.c b/passt-repair.c
index 120f7aa7..d36c9c94 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]
+ __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,22 @@ 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 +
+ strnlen(ev->name, ev->len) -
+ 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);
...ev->name not being necessarily NULL-terminated and passed to
snprintf() here. So I applied this with an additional (and useless, unless
the kernel is unrealistically buggy):
if (ev->len > NAME_MAX + 1 ||
ev->name[ev->len] != '\0') {
fprintf(stderr, "Invalid filename from inotify\n");
_exit(1);
}
That should do it.
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.