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 55A3A25670; Sun, 30 Nov 2025 17:25:18 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 177F3256A0; Sun, 30 Nov 2025 17:25:15 +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 fout-a5-smtp.messagingengine.com (fout-a5-smtp.messagingengine.com [103.168.172.148]) by atuin.qyliss.net (Postfix) with ESMTPS id B06302569F for ; Sun, 30 Nov 2025 17:25:13 +0000 (UTC) Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfout.phl.internal (Postfix) with ESMTP id 8F075EC0209; Sun, 30 Nov 2025 12:25:10 -0500 (EST) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-06.internal (MEProxy); Sun, 30 Nov 2025 12:25:10 -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=fm3; t=1764523510; x=1764609910; bh=cDZF8UzAmM Z3B6xnz2U8eXXSCKOFLQ7PHZh2YxEW/kE=; b=M6jZOLjlyKJpsxqmg+4vUknrqy OkI76lKvqnVRc26zMn1Vet0s1bze4AfBaiuqX/4jU5f9l+OauRgh12f4fk7PMkAC nMRWmIrOYyMBMVEKM0ATRbqwbWgNfQrskQYzcnLWCoU6aFh5t7maavOlPjINx3dM H+aLrfN0NMSzTwlBtmmMAW54LIyukUtCGkskvdfeiEZCUsGKYzfXx63KUQn6h54Y OLZTDTpwgv1KQJPntpebhdLaY1qw+L8NvPs/gl3tA+yLNXMf3K+eFqMu3paNNM+e mAOK32OYWUuWTdmvRm1amvseZ/67pR+x/Kb1vboaEU2bnsRX5ZLBzcBuB5hw== 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=fm1; t= 1764523510; x=1764609910; bh=cDZF8UzAmMZ3B6xnz2U8eXXSCKOFLQ7PHZh 2YxEW/kE=; b=MaAKgIeEGYXm1lWuEX3Bak8tmUkSeAtp6U4Cs4IhdKuDj4k4dlI evPwly1lyj4GwSCetCkS+vJKmPDMoHhvfO4zb8KEURmAiM4TouNjSywnBOYck4as l1/9mEmys0sOInXTxbK7l/aUXxO63rZ/kuWrWjiD0+0tS8svysvtm1nDF/jLjXHD roXdVv+9smRyHmmqklc0u+yWkt9Fy2xa18PwRe/GglqCL06PRnCkl8+T9SJxUQfx liWBBSwT6RfORh05+Q7mhsppG9tcfvoC79QI4FpqtbTIa3PYot9amWEy41xpv2iL l2rZVpgjkUzT0t4FMOPYy8ctcsn3cw3Gk5A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvheehgedvucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttdejnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpeetheevud fgjefghefhieejudelkeeljeegvdekueeuhffhgedvveefteevgeetieenucevlhhushht vghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehhihesrghlhihsshgrrd hishdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohep uggvmhhiohgsvghnohhurhesghhmrghilhdrtghomhdprhgtphhtthhopeguvghvvghlse hsphgvtghtrhhumhdqohhsrdhorhhg X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 30 Nov 2025 12:25:09 -0500 (EST) Received: by fw12.qyliss.net (Postfix, from userid 1000) id 07A10325D3DB; Sun, 30 Nov 2025 18:24:58 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v3 1/3] tools/mount-flatpak: init In-Reply-To: <03b28aa9-c2fe-49d1-8f76-83240d85ae20@gmail.com> References: <20251127202311.42422-2-hi@alyssa.is> <01179fd8-ca6a-43f8-b61d-447d507dd5a6@gmail.com> <874iqcekad.fsf@alyssa.is> <03b28aa9-c2fe-49d1-8f76-83240d85ae20@gmail.com> Date: Sun, 30 Nov 2025 18:24:55 +0100 Message-ID: <87345vxwk8.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: C3LISSBB6HUDRKJK6WO6YHIUKZ77ECM3 X-Message-ID-Hash: C3LISSBB6HUDRKJK6WO6YHIUKZ77ECM3 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: devel@spectrum-os.org 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; charset=utf-8 Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > On 11/29/25 14:00, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> On 11/27/25 15:23, Alyssa Ross wrote: >>>> +fn run(mut args: ArgsOs) -> Result<(), String> { >>>> + let Some(user_data_path) =3D args.next().map(PathBuf::from) else { >>>> + ex_usage(); >>>> + }; >>> >>> This should be an absolute path, as the working directory isn't one >>> that the user should be caring about. >>=20 >> I don't see any reason to restrict the user like that. > > The advantage is better error messages. A relative path will result > in a confusing "does not exist" error at best. > > Definitely should not block committing this. From=20what I can tell mount-flatpak works fine with relative paths =E2=80= =94 are you talking about the cd in run-flatpak? I don't think it makes sense for one program to reject valid inputs that might be invalid to a higher layer that runs it, but we could consider moving the cd into mount-flatpak by adding yet another argument for the destination directory. >>>> + let target_installation_dir =3D open_tree( >>>> + CWD, >>>> + "flatpak", >>>> + OpenTreeFlags::OPEN_TREE_CLONE >>>> + | OpenTreeFlags::OPEN_TREE_CLOEXEC >>>> + | OpenTreeFlags::AT_RECURSIVE, >>> >>> Missing AT_SYMLINK_NOFOLLOW unless it is is implied. >>=20 >> We rely on nothing malicious being able to mess with the target >> directory while we're running, and we do not create flatpak as a >> symlink. > > Still good practice anyway, but not a blocker. Will add. >>>> + let config_source =3D open_tree( >>>> + CWD, >>>> + config_source_path, >>>> + OpenTreeFlags::OPEN_TREE_CLONE | OpenTreeFlags::OPEN_TREE_CLO= EXEC, >>>> + ) >>>> + .map_err(|e| format!("opening {config_source_path}: {e}"))?; >>>> + move_mount( >>>> + config_source, >>>> + "", >>>> + config_target, >>>> + "", >>>> + MoveMountFlags::MOVE_MOUNT_F_EMPTY_PATH | MoveMountFlags::MOV= E_MOUNT_T_EMPTY_PATH, >>>> + ) >>>> + .map_err(|e| format!("mounting config: {e}"))?; >>>> + >>>> + let mut attr =3D libc::mount_attr { >>>> + attr_clr: libc::MOUNT_ATTR_NOSYMFOLLOW, >>>> + attr_set: libc::MOUNT_ATTR_RDONLY | libc::MOUNT_ATTR_NODEV, >>>> + propagation: 0, >>>> + userns_fd: 0, >>>> + }; >>> >>> Propagation should likely be set to MS_SLAVE. (Yeah, I know, >>> terrible name.) attr_set should also include MOUNT_ATTR_NOEXEC as >>> well as the host has no business executing anything there. >>=20 >> The host has no business executing anything at all from the whole user >> data partition, so this is not the place to set that. There's no reason >> this mount _in particular_ needs the flag set that doesn't apply to the >> whole partition this mount's flags are inherited from. > > Good point, but I still think the propagation needs to be set to > MS_SLAVE. Right now, mount flags are set manually by the user. Will do. >>>> diff --git a/tools/mount-flatpak/src/metadata.rs b/tools/mount-flatpak= /src/metadata.rs >>>> new file mode 100644 >>>> index 0000000..dfc05b1 >>>> --- /dev/null >>>> +++ b/tools/mount-flatpak/src/metadata.rs >>>> @@ -0,0 +1,19 @@ >>>> +// SPDX-License-Identifier: EUPL-1.2+ >>>> +// SPDX-FileCopyrightText: 2025 Alyssa Ross >>>> + >>>> +use std::fs::File; >>>> +use std::io::read_to_string; >>>> + >>>> +use crate::keyfile::parse; >>>> + >>>> +pub fn extract_runtime(mut metadata: File) -> Result { >>>> + let metadata =3D read_to_string(&mut metadata).map_err(|e| e.to_s= tring())?; >>> >>> Missing limit on the size of the file to prevent unbounded memory alloc= ation. >>=20 >> There are lots of ways to cause unbounded host memory allocation. >> Modifying everything to limit size is not feasible. The only way to >> prevent this is to run this stuff in a VM-scoped cgroup. > It's easy to run the VM's services in a cgroup, but running commands > like this in the cgroup is harder. Also, this command is much easier > to make robust against problems like that. > > Should not block commit though. I don't see why it needs to be. The cgroup will have to be created by run-flatpak, so why wouldn't it be able to run mount-flatpak in that cgroup? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCaSx95wAKCRCZddwkt31p FbcIAP9GcVRevxYZKf7maNm1OINC7pp+siIw+IlOOqwYJRMpugD+M/Zz9SP5alaU d33H4sqDkY4kUqDNFo1jwuc9cOI0xw0= =uMsR -----END PGP SIGNATURE----- --=-=-=--