From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id C2BD0207FE; Thu, 13 Nov 2025 18:01:32 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 182FF20897; Thu, 13 Nov 2025 18:01:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fhigh-a5-smtp.messagingengine.com (fhigh-a5-smtp.messagingengine.com [103.168.172.156]) by atuin.qyliss.net (Postfix) with ESMTPS id 7A86C20895 for ; Thu, 13 Nov 2025 18:01:28 +0000 (UTC) Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfhigh.phl.internal (Postfix) with ESMTP id 4190714000DD; Thu, 13 Nov 2025 13:01:27 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Thu, 13 Nov 2025 13:01:27 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1763056887; x=1763143287; bh=uY6oiUBdGs ZtyHUWV8ZLCnrRoNfo2XlPrPm9/6H4yqE=; b=aic76ZQCni6BxCFghVaC4SnEab 5RlYffzjYNw23/h6K8dGI3cqJns5SrlJ13a3PP9/eQXFDXeuhqyIN4nl/Y0KCUU2 ZO5gN2yN7JkoABRrAqLS3EgC5zBoJyjnI1xrGnZMoq0PZndFXGy+tToxzMDnelft v/N38kTZpeUZi59uc6Id5ae97j7CakPxVtP8JbiUf++L2B0GlB6nXbB3tZ9gWgjE TEvl4/h6zEXweVjUZphgcRNrq+Vib2YefbA/t6kXeIAI4D/LVp87MF9yu07VDyJP zGHEfXM0yvHJpl7CcrD1rWEAvpB4PRBTS6r+mIns3K/PI5AV6alNuwaKLYHg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1763056887; x=1763143287; bh=uY6oiUBdGsZtyHUWV8ZLCnrRoNfo2XlPrPm 9/6H4yqE=; b=WhyinfYmtfXR7rc+8ZvqCiA5E1HgMR6aFFaNyG7D31RIiCiys6t qlhGNKh5DD/Xq7OTFN+ptUrF5f17qIICuOtu/Wv2rqZZDPKpjTz2zY8X+5x57wy+ 7ZIIRb3b8x+IoTRtnFJiqA8a92j/izmg7YUFOdH4664no1Cpgf/DNAAEuE9meCWA TFUv7xUVxyACmYvvpqZ4kHEkI9EvHJrS6jg76AERBW2WPI9VCDKUmWHzSuf0B1IN TeBu4ztXbAYr4wjHTl2b9Qnoezz2y05sVNR9Rm1oZBFxJsrqo1G3K9Z8/oqI56N0 1C8grH5Nf8vlODOdZvsGyVz1YaXSUwENMNg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvtdejieduucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttddtnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpeeiudffue eilefgtefgtddttdekkeehkefgheekudefveetgeefiefftedvteeuveenucevlhhushht vghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehhihesrghlhihsshgrrd hishdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohep uggvmhhiohgsvghnohhurhesghhmrghilhdrtghomhdprhgtphhtthhopeguvghvvghlse hsphgvtghtrhhumhdqohhsrdhorhhg X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 13 Nov 2025 13:01:26 -0500 (EST) Received: by fw12.qyliss.net (Postfix, from userid 1000) id 80A3D164B585; Thu, 13 Nov 2025 19:01:25 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v2 3/8] tools: Add directory checker for updates In-Reply-To: References: <20251112-updates-v2-0-88d96bf81b79@gmail.com> <20251112-updates-v2-3-88d96bf81b79@gmail.com> <87h5uygjax.fsf@alyssa.is> Date: Thu, 13 Nov 2025 19:01:23 +0100 Message-ID: <87cy5lbymk.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: 5V4BPAGAD2OLFOSCDY2MCQMGD274MMVS X-Message-ID-Hash: 5V4BPAGAD2OLFOSCDY2MCQMGD274MMVS X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > On 11/13/25 08:21, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> +static void checkdir(int fd) >>> +{ >>> + DIR *d =3D fdopendir(fd); >>> + if (d =3D=3D NULL) >>> + err(EXIT_FAILURE, "fdopendir"); >>> + // If there is an I/O error while there are dirty pages outstanding, >>> + // the dirty pages are silently discarded. This means that the conte= nts >>> + // of the filesystem can change behind userspace's back. Flush all >>> + // dirty pages in the filesystem with the directory to prevent this. >>> + if (syncfs(fd) !=3D 0) >>> + err(EXIT_FAILURE, "syncfs"); >>> + for (;;) { >>> + errno =3D 0; >>> + struct dirent *entry =3D readdir(d); >>> + if (entry =3D=3D NULL) { >>> + if (errno) >>> + err(EXIT_FAILURE, "readdir"); >>> + break; >>> + } >>> + assert(entry->d_reclen > offsetof(struct dirent, d_name)); >>=20 >> Would be a POSIX violation for d_name not to be valid I think. > > Indeed it would be, but I preferred to check that explicitly. > >>> + size_t len =3D strnlen(entry->d_name, entry->d_reclen - offsetof(str= uct dirent, d_name)); >>=20 >> POSIX also guarantees a terminating null byte, so strlen() would be fine= here. > > I preferred to double-check libc, but if you prefer to get rid of > those assertions I'm okay with that. I don't think it's Spectrum's place to confirm libc is POSIX compliant. Adding these checks isn't free, because it's more stuff to get past to understand what's going on. >>> + if (entry->d_name[0] =3D=3D '.') >>> + if (len =3D=3D 1 || (len =3D=3D 2 && entry->d_name[1] =3D=3D '.')) >>> + continue; >>> + unsigned char c =3D (unsigned char)entry->d_name[0]; >>> + if (!((c >=3D 'A' && c <=3D 'Z') || >>> + (c >=3D 'a' && c <=3D 'z'))) >>> + errx(EXIT_FAILURE, "Filename must begin with an ASCII letter"); >>=20 >> Would the comparison not be valid without the cast? > > It would be in this case, but the subsequent cast to int in the error > path assumes that the cast was done. Signed characters are much > trickier and casting to unsigned makes the code easier to reason about. Is it safe to assume 'A' etc. are representable and comparable as unsigned values? (I'm sure it is in practice, but I'd like it if I could be confident this is being done strictly correctly.) >>> + if (entry->d_type !=3D DT_REG) >>> + errx(EXIT_FAILURE, "Entry contains non-regular file %s", entry->d_n= ame); >>> + } >>> + closedir(d); >>> +} >>> + >>> +int main(int argc, char **argv) >>> +{ >>> + for (int i =3D 1; i < argc; ++i) { >>=20 >>> + int fd =3D open(argv[i], O_DIRECTORY|O_RDONLY|O_CLOEXEC|O_NOFOLLOW); >>=20 >> Wasn't O_NOFOLLOW going to be removed here? > > It should never be called on a symlink, so if it does that is a bug > in the caller. I can remove it if you prefer, though. I don't think it's the place of this program to put constraints on its caller like that. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCaRYc8wAKCRCZddwkt31p FefIAP45NDWF/FlEWBIFTwyxBF4yQvNpQzRohkWdMsL6xRGX/AD6AinuNEa1B02G HpOIqBIoLY4V/bAT4WoFOcX7xPm8pgE= =14lN -----END PGP SIGNATURE----- --=-=-=--