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 7C1F6C04E69 for ; Wed, 2 Aug 2023 12:06:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ECFA1280165; Wed, 2 Aug 2023 08:06:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E58C0280143; Wed, 2 Aug 2023 08:06:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CF94F280165; Wed, 2 Aug 2023 08:06:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id BC6CA280143 for ; Wed, 2 Aug 2023 08:06:35 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 75D62B2746 for ; Wed, 2 Aug 2023 12:06:35 +0000 (UTC) X-FDA: 81079037550.13.619F92F Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf07.hostedemail.com (Postfix) with ESMTP id BA3D640024 for ; Wed, 2 Aug 2023 12:06:32 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="Jp4y/vBY"; spf=pass (imf07.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@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=1690977992; 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=KuMX+v6bwb/MoYVzYyRYHk3KTOEgERuvCaAUE3t0loc=; b=DMrv+0DxEsFtIKz5M5185OJf3qm84LjwDfDDVKg+zsATzgaQI/NCiRzpXRIXTYkX5A7r6y Br2xNGcF3Fqf9TJwmDBg8oWO2LOp9gFzEMMxcFc0otHuf0lNbviB4Un6qSGCCpXj/ooJ9e pl9z8NQFn4+bP8KX3JXhqlnS18nrq6Y= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690977992; a=rsa-sha256; cv=none; b=pgt4p5RS/7BxYOGjUqCpgeWA6StRfhpSy7KoGjZ71aDoYR3XnJ2joVQCt/vXIOk+2pGZgb O8cA817pZCrequNumhz1RFle2qsYeM7Nb91URgeGTuavFFiYRtSdgVXK/e+OSX4cfw+gxl a5j882XV1+fpvc1e2FtAi5WDPfEqEXo= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="Jp4y/vBY"; spf=pass (imf07.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@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 AA1E56193B; Wed, 2 Aug 2023 12:06:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C4990C433C7; Wed, 2 Aug 2023 12:06:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1690977991; bh=T9Tt1o6bzSJkt57u1XlP+azz7nrrUb1PBizrujFRBw0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Jp4y/vBYK7QgBWxlBbb86xnqrVQnKhv+2mAVUUs6T6Rs/1Uqgf2k+wI3z2L8H5cxH 8tLSJeaMU8t2fEsJkr0s2W8il5uMnIeXbYX4vLEJgmw2ktkl/qP6aI6M2l3frLEqh7 LcjBb3H603BPcYSLhENVNo6+4ux9m2weaUwI0IIJzV9LZ21GLAgO/KsPD8b6cMQJdG FEvhliptvHLyvEJ+YSVQPzwBiYj7MtO/edYOycsNae7CwYcz4RwHGmevVcDqvlql0p iQ944Ij5g6XLCZHKP3aFkpn7FSpMmF4GCEuIBtd8qr7BIe9IiYHMH+ZAWSYv+MuG9C nrNmc0zZHSwTg== Date: Wed, 2 Aug 2023 14:06:26 +0200 From: Christian Brauner To: Seth Forshee 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: <20230802-preschen-streng-9f2017794d93@brauner> References: <20230801-vfs-fs_context-uidgid-v1-1-daf46a050bbf@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: BA3D640024 X-Rspam-User: X-Stat-Signature: jscrn61o4391teh6smjjwuggp4arbzy3 X-Rspamd-Server: rspam03 X-HE-Tag: 1690977992-908625 X-HE-Meta: U2FsdGVkX1/HjEUvFj+/cGhfuHqLHl3WQoU1UKPDSLaL1Iutq4BaLmxJlreY617OytdG+tVXv7UqYKhyOy5fUTOHgTvMb+eD0InK4vIs9DR8q8d60E9KBw9O83LLqwoy00jqaWJRP2Y87YFQu7Nv/1CwBmorhWEyTa29Y4qgSl7TzZfyfB3LyUiWlSXX3JEdYRMWk07pkum36Y4V/1Fq+AjtrmU63qghuK9iThIdI5UVaHn9FoXDmrghcsAfnEIeRf1yylLHv+X99AJX29peSwRxOdsejL3o4NOoQlTAyrkMZ355w+I1VwexYMWWiTrhKb3yP10TDm/v+ZTA/bpxdbJeUnh3ma+ald5PIaFZnd+or/VyFkIoS6Q5svP4cUTgAtAwf4miCAxokDa29me3Ju9J7fsBegHmUR/OC8I9z6jQ1OnuJ6SasjW3OLzHJjF7ofcojAjnEEHkxS9FZxsEYJGP7L4FmvEZcQhss1HRkAggMndWU+M3dHW4MOzzGMI+YmjD/VD0vfWpn4D+BYBqDG0oS9o9FYJgvP4txWNC/Xyp5QRyQVp24GOsVN4wtONa0L8ei8GtHB6wfRfgvedWaHpu/QsHXdOFCU3cNEXaQHbiW6rZh32mmFgxLRxlwZo8P4dzvqS8uMr1adYQtstZAdILOLVIkbZfZ8/px/gV9lrr9nbEmmvPNC8YHf/8OqSshDXeF2GF+zeX2wbsy9dY+7oBnP86qtkTOei3gMVGV2lvZPhPwL5hYGob+vCmER1dGQBH9fVYFqbWVb+e82L1wx65d0HZoNCJBYYfGgpRsWT3mneViwWQo/sdC5RNJ/hV76DaAOBS+g4rP/JyJlekCxzpTG8lnGsERZHaz2ArlG1ziRehmQkUeI+KUcQZTRNPwA48khwUNCHJhvAprY96CCgedIGFRj1aEPvGZOH+Yl3kF9338KRwJNHVKXnoQDDZY7oYXlz2IWIRdqFv00B xzYj6MWA AQnh8T1UPTrLOkOrJdWup9jgNyrkpOIBg3OB7Dq3qXAzmStonYVyPioUrmcEnMOFdkt5Zcvcd69J7wf6768GBwL7qj85PxfzKe2qXzTMDxh9eNbZeyIQZK7z1dACdcOsAJiuVMIPU71L6U8dIgzfHuM7yqUIjYIuqIRX+DbQclj+ULKnHh0cOSVkYDhoj4ZyEqeKbOsGk9rM/9H8FJQ+WTMBBXqcBI0lrC7MlsvU807ioCeLCqw1SF3yYEEx6Svz/W4QNaWQ4doug38E479wTHSs7qdMzCAamM0vI1vcD2C1T7dmdw2roFOCD/z2RFDXYZj8sDMoRWTRFTYC2ZpicSzv4mRXS+iTNxdvFfj3kNKFlidLeFjQ8yZlJu4NrYpo4KsQ1DrznIZ4aPC8= 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 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.