From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28CEFC04A6A for ; Wed, 2 Aug 2023 13:45:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 259DE28017B; Wed, 2 Aug 2023 09:45:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 208D7280143; Wed, 2 Aug 2023 09:45:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0F82328017B; Wed, 2 Aug 2023 09:45:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 015CF280143 for ; Wed, 2 Aug 2023 09:45:37 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 7DE40A0ED5 for ; Wed, 2 Aug 2023 13:45:37 +0000 (UTC) X-FDA: 81079287114.27.650069B Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf11.hostedemail.com (Postfix) with ESMTP id 55E3840028 for ; Wed, 2 Aug 2023 13:45:35 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=I5q6Uc9u; spf=pass (imf11.hostedemail.com: domain of sforshee@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sforshee@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690983935; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=RutTybPP8061+vOwGzIhzuARQH3biPxa8/bElR01MI0=; b=BbPb40C03L4Vvx1/MMbjtbr2ImfBY4k9KDZqEUqZX9cwgMgCbuKaUoi2q/uXLkFnB542Hk TJITt5cFIOQaWe4kCLwqppG8LAc8GmlsyzvqYiZSgNGtvmDEKE5dIND08AZarO5QmNKpay kEvLHs8ld7EbSWPooxw0MwzQ3LIJiTg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690983935; a=rsa-sha256; cv=none; b=8AQNDJTiPmE0CGK5cL2UJQNJ3B1+StVdZPhLPpAE2tSQVrdJc9hjOhuSEoVBIRRlvgVE9K 9nDkK6iscjb/8HvcDUb1+luCQ2lJEvlk17n+EGYjZAUKgTSArYl7hbAEePSzqywXFAuReu 51q0v2sOjosCOH6fxaGTTkSRo7LokTk= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=I5q6Uc9u; spf=pass (imf11.hostedemail.com: domain of sforshee@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sforshee@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 33FA86199A; Wed, 2 Aug 2023 13:45:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7DA36C433C8; Wed, 2 Aug 2023 13:45:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1690983933; bh=7BQyrIhNLZeMEk47Y2SNYbZTj0iSs/HO5S+sU/9nDvE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=I5q6Uc9ur3y5bY7AJPnT7asxzcSfd/m7Oe30+1zAyamlLcBVA275k+APvyuQccuuD N268Y9/5Pt+ReqWQ3ig+w2Ml1BSrnezq3Hb2EcfUtSR2QubHD0XSy4eMk+4wPHXlPb MXU5ku0iCGOV2tV05TMIWy8VsJeF2Q8nr3Eat4s5v15Zi8gF8Cry8s4o/UX+qoSEZJ gWMNuW6PKaOYYJb4nRYfdoCv50QFDrTPiLiP+qfz5Fzdx2ode8Et6Hxp2vR9+Yq6xX zZQg9WaU4S+AGAoIKnQ3RYH1qx34rwr8mHHwM4WWb2XbGxMu7FFHm5y/1i6bnQaXB+ 9zc6RGBy7nGCQ== Date: Wed, 2 Aug 2023 08:45:32 -0500 From: Seth Forshee To: Christian Brauner Cc: Hugh Dickins , Seth Jenkins , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] tmpfs: verify {g,u}id mount options correctly Message-ID: References: <20230801-vfs-fs_context-uidgid-v1-1-daf46a050bbf@kernel.org> <20230802-preschen-streng-9f2017794d93@brauner> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230802-preschen-streng-9f2017794d93@brauner> X-Stat-Signature: oiotcw7up3g1qpts816mntkjy57if83g X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 55E3840028 X-Rspam-User: X-HE-Tag: 1690983935-550331 X-HE-Meta: U2FsdGVkX19WCEXfSBhiYn64YOC7Cx41CKHlZRSvVoNs+B7JlN22dE7khPfQaYMaSrMyehGzouWBCbsJnWe5KHW2rSBJt0+OrXROiIJpzafIWAm2QrRQmGETI24WzkCpLNYgnTUbUq2HauoUm3RJcfxmnfbHckz6KIJt2qAt2VG1uny2MsQmzNGTC9BO4ZrnNlz17GH/7U+OnLKcDheW+pMH+i8HdMh3N+4booOcvZrJTkO0L/GXDCiTGINRe6FYXmm1grChNSu1VuPSP2I49OBVFr4RokISrRotOpQlqgV29ISsbE/RbTsSLFLfw1p1NTftrm4Ilu3WfTPjtG/eAkb/wxatJlo/wQyVUmSrIUiz0YZwe375kbNk9AIzZ6tS7a4ER82aRzWc6GKeFPnNxse56kWHrze81ai4hUOH9TMgru2nh6YiPf/Vt0lbbQR0SGoCQ+OYepb8wagZ6u+frb3QDg+Qob0duYvfV9z62xYZH6pYFg5xDkMb/5grU24oDq+obxv4FqrBERrK0bL36VyKEqYsTkT/xb8407+/i1FVufwwTwbWknE9rd3A7KPPCu+/VC1zD1nTRsSNvBCG2i2cRbQ5WAbixbYRVxS8UNV8ja7WLtlXwDTBqOfNrciA8PwsHiYlzFb97alYl0ZZ4hXxw2c/5By/KMLMuh9qM6o4Ius6K0kZPZww0kLF2DywOC3q5vLy0MqOON22QNUdWPgpGlnhw6Pob8GYW4Pt02gG7obAa4Uaqojua7ebvP/Ud9upiB1/tam29I9DQQvd6cpCHPuDLnQBBWHC36lYgwjmREl163uPBJeNQ4y8BDLB3bDcnDhFtjo/uzSGg8P0J6UKxTiIDBt7w7x7K8622alORLFA6gZzisZsU/Jr3Atbv+oHjmFg32r0hbeopob9dXvFYazE9l6DhshvWyYNaF5BN690wEBpXAiu0sPm80yDSX5dLQnNphEletXMChA T2AyRuGF e3v7c2ZN+u+vusr3SdiUnm2ysS2AWK8F0D69FdRPCZb7hhUDxZB63Y1VPOwIs3dt12w5VvFgsc+/8gK+k39fSvp9JJHygHEQDP5i3fOyUidxUO9LTnz5FNVD7I+vtixJAqW7MDqZBZYLKgpFikhyth71Oqon9WnQW1EojW5NTvE2wUdQ2AKN1ai6kEUY9YCVV1Z5HFolWA64WgMqrAgE/xFZcemCwjVlHnRQbdpJQNnnri0l0oEcJewWFw+YrPaD2Oij9fikMccqA/0X3sWIu//aZJPV8P6n68YYONq0aOOyWkzwcVWXS7A4G7V+QGlBvi8kpZP0EHSjOmLmUE763xfYCNx+DlWGR0PgybwVKZogabBZqjX7HyKUW4fAUcTpgE+P2sg0VvG0KhMc= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Aug 02, 2023 at 02:06:26PM +0200, Christian Brauner wrote: > On Tue, Aug 01, 2023 at 11:47:41AM -0500, Seth Forshee wrote: > > On Tue, Aug 01, 2023 at 06:17:04PM +0200, Christian Brauner wrote: > > > A while ago we received the following report: > > > > > > "The other outstanding issue I noticed comes from the fact that > > > fsconfig syscalls may occur in a different userns than that which > > > called fsopen. That means that resolving the uid/gid via > > > current_user_ns() can save a kuid that isn't mapped in the associated > > > namespace when the filesystem is finally mounted. This means that it > > > is possible for an unprivileged user to create files owned by any > > > group in a tmpfs mount (since we can set the SUID bit on the tmpfs > > > directory), or a tmpfs that is owned by any user, including the root > > > group/user." > > > > > > The contract for {g,u}id mount options and {g,u}id values in general set > > > from userspace has always been that they are translated according to the > > > caller's idmapping. In so far, tmpfs has been doing the correct thing. > > > But since tmpfs is mountable in unprivileged contexts it is also > > > necessary to verify that the resulting {k,g}uid is representable in the > > > namespace of the superblock to avoid such bugs as above. > > > > > > The new mount api's cross-namespace delegation abilities are already > > > widely used. After having talked to a bunch of userspace this is the > > > most faithful solution with minimal regression risks. I know of one > > > users - systemd - that makes use of the new mount api in this way and > > > they don't set unresolable {g,u}ids. So the regression risk is minimal. > > > > > > Link: https://lore.kernel.org/lkml/CALxfFW4BXhEwxR0Q5LSkg-8Vb4r2MONKCcUCVioehXQKr35eHg@mail.gmail.com > > > Fixes: f32356261d44 ("vfs: Convert ramfs, shmem, tmpfs, devtmpfs, rootfs to use the new mount API") > > > Reported-by: Seth Jenkins > > > Signed-off-by: Christian Brauner > > > --- > > > > > > --- > > > mm/shmem.c | 28 ++++++++++++++++++++++++---- > > > 1 file changed, 24 insertions(+), 4 deletions(-) > > > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > > index 2f2e0e618072..1c0b2dafafe5 100644 > > > --- a/mm/shmem.c > > > +++ b/mm/shmem.c > > > @@ -3636,6 +3636,8 @@ static int shmem_parse_one(struct fs_context *fc, struct fs_parameter *param) > > > unsigned long long size; > > > char *rest; > > > int opt; > > > + kuid_t kuid; > > > + kgid_t kgid; > > > > > > opt = fs_parse(fc, shmem_fs_parameters, param, &result); > > > if (opt < 0) > > > @@ -3671,14 +3673,32 @@ static int shmem_parse_one(struct fs_context *fc, struct fs_parameter *param) > > > ctx->mode = result.uint_32 & 07777; > > > break; > > > case Opt_uid: > > > - ctx->uid = make_kuid(current_user_ns(), result.uint_32); > > > - if (!uid_valid(ctx->uid)) > > > + kuid = make_kuid(current_user_ns(), result.uint_32); > > > + if (!uid_valid(kuid)) > > > goto bad_value; > > > + > > > + /* > > > + * The requested uid must be representable in the > > > + * filesystem's idmapping. > > > + */ > > > + if (!kuid_has_mapping(fc->user_ns, kuid)) > > > + goto bad_value; > > > + > > > + ctx->uid = kuid; > > > > This seems like the most sensible way to handle ids in mount options. > > Wouldn't some other filesystems (e.g. fuse) benefit from the same sort > > of handling though? Rather than having filesystems handle these checks > > themselves, what about adding k{uid,gid}_t members to the > > fs_parse_result union with fsparam_is_{uid,gid}() helpers which peform > > these checks? > > Yes, I like that proposal. Let's see if that works. After a little poking around, this is more complicated than I'd initially thought. The parameter helpers don't currently get passed an fs_context, and ceph/rbd seem to be using the parameter parsing like a library when there legitimately is not an fs_context to be passed. So it makes sense to take this patch as an immediate fix, and we can take a look at trying to make it more generic later. Reviewed-by: Seth Forshee (DigitalOcean)