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); } -- Stefano