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 658D7C00528 for ; Tue, 1 Aug 2023 17:44:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D177E940040; Tue, 1 Aug 2023 13:44:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CC822940010; Tue, 1 Aug 2023 13:44:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B695F940040; Tue, 1 Aug 2023 13:44:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A61FD940010 for ; Tue, 1 Aug 2023 13:44:38 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 5F3AC140493 for ; Tue, 1 Aug 2023 17:44:38 +0000 (UTC) X-FDA: 81076260636.11.B6BED56 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf14.hostedemail.com (Postfix) with ESMTP id 292AF101AB9 for ; Tue, 1 Aug 2023 16:55:03 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mgKUuWYk; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf14.hostedemail.com: domain of sforshee@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sforshee@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690908904; 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=/Y2PaEKp1nZxYc5qK6V2j8AtfW7AJ1poQmTe+14FQNo=; b=QP2rUKnalqmC6vp6bQ8NADJ9i7GVG9jmiEnwR0dWcx2vowButh7nxCxT/Ki9PzBSgjq9Hg GxHSeiKIsoGQEc9s5bu39wW1+VLO4vtqQERJ5y9bpUZkYvzvRzR5n6+fkuLVtRsgYMpVv7 po+nh7fkPGVdyBcD+qK8fi4ohPfXSDk= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=mgKUuWYk; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf14.hostedemail.com: domain of sforshee@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sforshee@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690908904; a=rsa-sha256; cv=none; b=7l56k3q4xWFRT8G5R21uUeiiSzj+ZLEcmAIjCgslK59SprbVZq9ukdFAVmmh9RGoQDxYM+ 7mDAlEhHFMr4HgoZHf9Yrmz8/nUdwNvX04IUv1gi5pU1GPCq4AhVreerBISxbPsq7gAKES pxnmb5w/nqgiMRjTqQXZ3eqjkQ8lyk8= 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 B4FE761617; Tue, 1 Aug 2023 16:47:43 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 01DEAC433C7; Tue, 1 Aug 2023 16:47:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1690908463; bh=2IkzLGM+Oibmn+oBwcZbKSgr/JI+miPPejQ79yPTHuA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=mgKUuWYkjeUsTvt1tTNz957/JJ4Nr8TXWyQzOGYoINgI5Tt449xAZzALNOmVufMHE puVNj6u8J2qeSj/aZElmiVnFBui6pkaxNGGCbIRGWrkGdnji405xvn31gFVkzXWVGg vWbgqbsg9cZ8S3MRFBi7zzRFS0M59/M5DGE2yuZhNPTH2J9QLzlVf20Zqa53l063R9 KsWCHNPCAe4r46hIB5oecfDACd4pxsdcG4W7Sm/JWtOUMBaikw3gSWH4WaaKIBE64p mZH4xI+JfzPL1qRfgZquvpTY0pAO39rlNZ64v7SZfz22UmnuBhteSzocO1p3s4rnGq tlLXv4OEkukZw== Date: Tue, 1 Aug 2023 11:47:41 -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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230801-vfs-fs_context-uidgid-v1-1-daf46a050bbf@kernel.org> X-Rspamd-Queue-Id: 292AF101AB9 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: aqjddzr8t3aif6rj4inc8duji7m7bmgr X-HE-Tag: 1690908903-912421 X-HE-Meta: U2FsdGVkX1/278crELGSPSJw6bDB0o3/LAQCrUAHsmSX6sEITjszi3553xfvCM1yID6Rac3L2VyZ+mBBImG9O5KDQ1Mcb3xM+Nc/M7EZ2IBXOdDMJiyKNZMq7q3GPTMMCDzV9AsgoD5M6WBbdrHkFC8R2w0ESZ4ubnIWh5WMA+SVhqbY4uvLMqWvuKPYiYLP6RPWdTtN3o3Ca4S5M8Tv2Npsf5dpcOPoAGSI8hWSb0PwnBtKl11qUmCfPz9NPOVOKyeaywUYL7Hn0nt6TgXi0LQP6PwvO4w6bBslK9HQ5QbAAsGrskTgNi1YflcnKAdfWYOOGed2NZFBY+sApC6rbBwB5n4k9Ho+qHoBxwGcu5Rb1vhz7kpSdH5uSHdc+P6X1MBqPrLaiIw/LxS9MPVUmwHHrBcZYRdk0yXtut6OySbZBTKDC1+eAIXH5nFPtjHuHuX8OzyXjpA1YzLfxBnHoQJNtVmHWttDmIgoMvNnQRqDPvbgBS+IRPg4ReWp/gPiq+13wxMQhczYCV9N2VQGU7MjH6GkzB2LgqgfwbRy4cfxhKquhhJUeVIKuPpI3m/OgIzYZtZRthVAXeNJncbEHBQoEytFFdmqfmA8BAcIL4agc+B+hr2eHytOvihNn1mMQYOvEUBT4qHEWyv4fW/I4MHJ42OdGy8jpegtig+1UE4LinW4dnjOyn5oE3Oagmk24iUNosTbL/xLVaGB1AdGJlosaWGqMan04r+rhPsdaTJAMaR3VfOS9q/CiQIkuFOgy87t+A4yRzwniJdUWsCEEdeMPxt05rmYe87BL3ABztWZggHet/P7Pz/Ngv7wUIX7aeupP63mC4aELV51x3bqPR2KIzgABbdRngN3oTY7LXy+aObpGV1VJyx86gNWLCOnWaNdMrg43U2vpmfKPmKnVJWkLzZLajGF0QpErbNYxCEMFzaDDTkm3ZwppQZnO06mCgJKaErOaMMf/WdZwZn 4RRRcEwY CGc2NcMq6buy7rBUDWZAnN1yT2BtXsQDB2EB92X6JYJwi0zcfQ46P5fxaOHP4AwO6Ix9ZKLL9vgLD5oWfXfkFztbL0qAf4+ttJ16xsTcT+QvNLJ7BrVrTQR7ss3YGyuc2OAZ7EA/ZjmC+f1yLfrKHRwwMZ8nX/ckIm4xGfxsomwWHhp8eofc1V23ZOF7ReCAWI4dblCnR0JlxNET95Dxd33M5U9ExL0WhuiOfJVv75tl0S3UIesU8J+ROEPjZk6P7JX6HcGSBAMQB8d+vz22Qq+GhfJMgPrRD3vb5eb2ahNjmXIN4QLQ0k7vN6bBIF96qyHtjSaXfCKKK/Fb8FqbSSiqIbP4JbsMpSfmF/sTc1Knz8I07ddB+2rTumotiy6Hay4NJa5f8yU9CkZo= 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 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? Seth > break; > case Opt_gid: > - ctx->gid = make_kgid(current_user_ns(), result.uint_32); > - if (!gid_valid(ctx->gid)) > + kgid = make_kgid(current_user_ns(), result.uint_32); > + if (!gid_valid(kgid)) > goto bad_value; > + > + /* > + * The requested gid must be representable in the > + * filesystem's idmapping. > + */ > + if (!kgid_has_mapping(fc->user_ns, kgid)) > + goto bad_value; > + > + ctx->gid = kgid; > break; > case Opt_huge: > ctx->huge = result.uint_32; > > --- > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 > change-id: 20230801-vfs-fs_context-uidgid-7756c8dcb1c0 >