From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202502 header.b=ZSmxdmQv; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 5549E5A0008 for ; Sun, 30 Mar 2025 07:58:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1743314321; bh=9ITC9jV8WPNA1ZmUxuff05+THMFSZFe5w+zPt+rC+m4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZSmxdmQv0i+mHlq5ibFLUGJ30AqpzasIn9J/el9WefpzSs75cv5art6SGwyjhyzr6 8kREXnHeEFGr1804gXSv3IpUqM7zRkX1AYa4ZK0jwdcx7aeCEFfAbKMZYRlxSrGqD4 fEGsSFcz+A5JXer3P45Y7iFwLyg5KikqHucBeSpLKlbq03/Z9NfXanrmaBxolrJclg q+gKq/eX9HCA1Kr8CNHN6qlNaqGCa1dosDj8hLYICeVUEyVjgO+pRKvx+wdYLUd+n4 bT6ql5QHNnspL7VsgvnJHqJrpQ8ye4gZX5cbYPxXY5LPz9sG8jLmDPeouod8xqlDZh zrUaXY9cd/MMw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZQNqP3Jwpz4wcd; Sun, 30 Mar 2025 16:58:41 +1100 (AEDT) Date: Sun, 30 Mar 2025 16:53:54 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2] pasta, passt-repair: Support multiple events per read() in inotify handlers Message-ID: References: <20250328003958.3570775-1-david@gibson.dropbear.id.au> <20250328152910.19365ba8@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7oK1TmHZ+YDEsxyU" Content-Disposition: inline In-Reply-To: <20250328152910.19365ba8@elisabeth> Message-ID-Hash: TTQLAEC3P5IQRF7UDMR6SUD3LAHZL3Z5 X-Message-ID-Hash: TTQLAEC3P5IQRF7UDMR6SUD3LAHZL3Z5 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --7oK1TmHZ+YDEsxyU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 28, 2025 at 03:29:10PM +0100, Stefano Brivio wrote: > On Fri, 28 Mar 2025 11:39:58 +1100 > David Gibson wrote: >=20 > > From: Stefano Brivio > >=20 > > 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. > >=20 > > Add loops in the two inotify handlers we have, in pasta-specific code > > and passt-repair, to go through all the events we receive. > >=20 > > Link: https://bugs.passt.top/show_bug.cgi?id=3D119 > >=20 > > Signed-off-by: Stefano Brivio > > [dwg: Remove unnecessary buffer expansion, use strnlen instead of strlen > > to make Coverity happier] >=20 > I have to admit I didn't know about strnlen(). POSIX.1-2008, that's > bleeding edge stuff. :) >=20 > That wasn't actually enough though: in my Coverity scan, it was also > complaining about: >=20 > > Signed-off-by: David Gibson > > --- > > passt-repair.c | 27 ++++++++++++++++++++------- > > pasta.c | 20 +++++++++++++------- > > 2 files changed, 33 insertions(+), 14 deletions(-) > >=20 > > 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) > > } > > =20 > > if ((sb.st_mode & S_IFMT) =3D=3D 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 =3D false; > > ssize_t n; > > int fd; > > =20 > > - ev =3D (struct inotify_event *)buf; > > - > > if ((fd =3D inotify_init1(IN_CLOEXEC)) < 0) { > > fprintf(stderr, "inotify_init1: %i\n", errno); > > _exit(1); > > @@ -130,6 +130,8 @@ int main(int argc, char **argv) > > } > > =20 > > do { > > + char *p; > > + > > n =3D read(fd, buf, sizeof(buf)); > > if (n < 0) { > > fprintf(stderr, "inotify read: %i", errno); > > @@ -138,11 +140,22 @@ int main(int argc, char **argv) > > =20 > > if (n < (ssize_t)sizeof(*ev)) { > > fprintf(stderr, "Short inotify read: %zi", n); > > - _exit(1); > > + continue; > > + } > > + > > + for (p =3D buf; p < buf + n; p +=3D sizeof(*ev) + ev->len) { > > + ev =3D (const struct inotify_event *)p; > > + > > + if (ev->len >=3D REPAIR_EXT_LEN && > > + !memcmp(ev->name + > > + strnlen(ev->name, ev->len) - > > + REPAIR_EXT_LEN, > > + REPAIR_EXT, REPAIR_EXT_LEN)) { > > + found =3D true; > > + break; > > + } > > } > > - } while (ev->len < REPAIR_EXT_LEN || > > - memcmp(ev->name + strlen(ev->name) - REPAIR_EXT_LEN, > > - REPAIR_EXT, REPAIR_EXT_LEN)); > > + } while (!found); > > =20 > > snprintf(path, sizeof(path), "%s/%s", argv[1], ev->name); >=20 > ...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): Ah, yeah, I saw that error but wasn't sure how to fix it. > if (ev->len > NAME_MAX + 1 || ev->name[ev->len] !=3D '\0') { > fprintf(stderr, "Invalid filename from inotify\n"); > _exit(1); > } That should do it. --=20 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. http://www.ozlabs.org/~dgibson --7oK1TmHZ+YDEsxyU Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfo3G4ACgkQzQJF27ox 2Gdvfg/9FuEl3m3FO7VvC0pQnTUpNx1ee9l3ORcWiEFVdPfpH8GlqDGaHsSmNiK5 btaKgjSkttJ9rPRNPxIH9CWXA9tBpjThhfILEKieu0mea+aj+sHq/O//err3NBIB yGkM5C/zEteqaT6T+980z9GyPJP9IY4YS8G2uCMxiTp0ypMoj3CfKzFphj2OVboQ llsj7kDVgyzXHmw0mmJ3vbv2vvO3VlZeIEEoakhGJBOA8ANxP0FLqvZB/2olgNFf woSLeNOZiSa8w5TnBjxHReT+4zdBTAbTfcEbS2IY5/8DjsK9HO5bi6avK8lW+9vA aD/qWYb6rv6Hq/8VBqftZnxhVFcKbvQyAh8CzgIUuJk+XJxpD3mhwXqe17/W/S7Z kD3wLZHwYp7S2kzBjetGSQhfg+YJtpZTAVPJDCc7LoniPAnQLYdQv6cXA0JaWx8J Fsi4fWLMOAiB/Cj5T0uyTV/5wYPlkt2Q2ZOfJFhemdbTtfHEwNJcZTExlQyagqq5 YH8EXMseBNwa1kgR5P6GBRA1ACb216SDhbFLcypQBM0rdVhpOYgku35modbtGS4I O++av9RqGzAhwxluOzlLIXjZWzOhqQQHmGNqwklT4Ddq9MfTG9AKA4M7fwqyFcKL MWXiR8gzVhwLRY+98nr4/Ry6eBX9pgPPTM+rzNDvNAjQpWDQdQU= =zPjM -----END PGP SIGNATURE----- --7oK1TmHZ+YDEsxyU--