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 6F171C77B6E for ; Wed, 12 Apr 2023 10:00:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EBDAF900003; Wed, 12 Apr 2023 06:00:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E6DA5900002; Wed, 12 Apr 2023 06:00:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D358F900003; Wed, 12 Apr 2023 06:00:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id C17F4900002 for ; Wed, 12 Apr 2023 06:00:38 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 01180B191F for ; Wed, 12 Apr 2023 10:00:09 +0000 (UTC) X-FDA: 80672293380.29.2ECD293 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf28.hostedemail.com (Postfix) with ESMTP id 12BA7C0005 for ; Wed, 12 Apr 2023 10:00:07 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=crNVkqcx; spf=pass (imf28.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=1681293608; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zcrlHy8RYZuT7jDiRT5cWBxdkdylehPdr4On+5VLhYA=; b=QIIIDmllgEFygICnD6bC3FUXHVrNS3A9azYI+fCLODmk/0o5VXdx8l2fHJhG3BLtduNvwf 60P4BYj7stAgdSYTgH2ytckiyTEcR74yVGEE54XPFg5Wq4S4YQW1f+lZ2ONx7n4i0vyauX IaaJb/Mo6stk6TDx6Je4/bs0L46hUnM= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=crNVkqcx; spf=pass (imf28.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-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681293608; a=rsa-sha256; cv=none; b=z0Glim+l/qL5Eu7bWctTWzCZot0dMWmJKbmTe1qYpo/T6hd/DmFC+aKwWlAJj5tD9L8rsD ChDIIasEg2G7/8ttPDhLXVQIxp5uxo1kV15aIDwEHuXHI9orwFZM9M7ZzvCQXWXbmW+NvW MK/NobjKrnp1j7TvkgJ9akulZHmpYyw= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 044AC62D39; Wed, 12 Apr 2023 10:00:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 00809C4339B; Wed, 12 Apr 2023 09:59:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681293606; bh=OUD03e+7Iy4QMxz3xJBYzLJlFWfJDEAkzkfLMPair6A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=crNVkqcxHCCYL80wSlDenZejktN87UpM/8LK4etPla1ppaaBuuLGirFySD+9BC8KA rok+vJxk0HE5dOvhmCScfvmfwOePw1RpVNIHWcNtjR8/OdKMSLM8epGate+sVgwibT 5g+HJMDJAqTuD4tNXDMa0mdfzwpVYJP21+59Ok0BLQ7p83MVc5+U2FahQ/OrlB3E0c 3+nXL8j94+wzyND696UMlIfan6ehWljHDgvm/lA3rvxTNr5GvpkPZqTGm9cngV+VsN CJD1pJXthpF+iTotF1i6mXyGtz8SKPgaEU1iVG4BxB/UHaVj28tiyTa6p0gZEsOF59 p48RQwfvtj8oA== Date: Wed, 12 Apr 2023 11:59:52 +0200 From: Christian Brauner To: Ackerley Tng Cc: kvm@vger.kernel.org, linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, qemu-devel@nongnu.org, aarcange@redhat.com, ak@linux.intel.com, akpm@linux-foundation.org, arnd@arndb.de, bfields@fieldses.org, bp@alien8.de, chao.p.peng@linux.intel.com, corbet@lwn.net, dave.hansen@intel.com, david@redhat.com, ddutile@redhat.com, dhildenb@redhat.com, hpa@zytor.com, hughd@google.com, jlayton@kernel.org, jmattson@google.com, joro@8bytes.org, jun.nakajima@intel.com, kirill.shutemov@linux.intel.com, linmiaohe@huawei.com, luto@kernel.org, mail@maciej.szmigiero.name, mhocko@suse.com, michael.roth@amd.com, mingo@redhat.com, naoya.horiguchi@nec.com, pbonzini@redhat.com, qperret@google.com, rppt@kernel.org, seanjc@google.com, shuah@kernel.org, steven.price@arm.com, tabba@google.com, tglx@linutronix.de, vannapurve@google.com, vbabka@suse.cz, vkuznets@redhat.com, wanpengli@tencent.com, wei.w.wang@intel.com, x86@kernel.org, yu.c.zhang@linux.intel.com Subject: Re: [RFC PATCH v3 1/2] mm: restrictedmem: Allow userspace to specify mount for memfd_restricted Message-ID: <20230412-kurzweilig-unsummen-3c1136f7f437@brauner> References: <20230404-engraved-rumble-d871e0403f3b@brauner> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 12BA7C0005 X-Stat-Signature: ft1kfafken6nik81fmrf8ie7r31id8rz X-Rspam-User: X-HE-Tag: 1681293607-401176 X-HE-Meta: U2FsdGVkX19yzajb/dIDbS+gNF6Gn3g+ZrvZXCpLZAoxz3QHvdVZOMvBFpbTgy6NbvOqMGl0FbCoA6tzrei+jjm6UY7WmRM+w0dwZwdm1w6+/YBVW3FpZODG4Gu49aRNwNEaZLAQNQJ4Ybwx1mAShxB+b/FJPYjQuQrlLh2zz8VQmEXoaJ0+Vm91n92x3kvn75wdbGTVvaMMYooLp6+RVlG82B+HlJiJKQV7TgxUR4D90poyE/COGiDGokrYi3Swnc62Ny7f2vnoqPK5wP8Snc6cZbweuchtlDCjG8oQtdCR/GdTfOFhNyVdJGx1TKlxqg6U3iUgH95EXfyd3Y3H7DcWml5iHnKkl0GFy1t9Fb4cm9E9bJz13QX0nGpPbSApqLeTiqG01z5CS46kIlct+qLCwB8neuX8nN+45rd/zqucGdotQPx5BzLOA4WedqJrgbgTEA+uzBjjJLxnWlW1duGYYGnQinDUBd0ZAHYmH6d21L3Cn73mT8GxCQo5Wwf1soJb5JhaML3uN4en2rbw98Vq6eYZ03Tju0j9hBUdk9DDK74WxBJngEbv+4h9SvkYsDxzXEu0Q4BLkogccL2nAVc6zd9IkipJyMx51a/C8z8ug2Evz2Ut9/s0d0cqrcusWtqNbaz+Ld/FJ7zM3Y7T6gqyIAiIWH2sFxgSShw0D7AFev1vKxvyW9kPyx2IesMps7ptPgMcG1/Wyb3m1uCG6jNdHMHJU4Jg2E5l4qY1HcPRxPYO0hU+OpFZKiUXQP5wTXU1UvOJHcLDR3LVJn7AZF/p7v5rXXvbCqDQrW+S9BgPzkKNJw4kZ56GeEpJiv9ssN4TrN8atRPU4zwpypAm0HplF+zMvvCKCE44wp/vJWDVSMvQZArcK+H0hiAEmLNDoaGoB5tVQL/UgjpHIjkVHIv47UsbzS9yMCT/qkGWgr3wrYzZFCGT7ok1XXanN42yvIqb3jj/1604bl3j41F zAhgrwAx 0AaJd8frHvt9pYlcPo3IENNoZCrBK5nLi+lDQGULhNgoVaDa6nPM4R44D4KHx8HmRZbFroXH3lryzcDmDaJDXXkN2OEVYTrmUCQsSL0MzzCD+/jt64q/qAqlZBH3MeL1LuPB8EGvqjDSgDdY4bqW1M0wCnOY7qnW4VVif0/yHdqzFLdQKDJYw+2C26gtlijJp7jpCZmj6TBAAsBbPq4KdYzqxfEyqOsMu/AB2IkHQpSSoYBSG673r56UoBbqWOrnfwHoX5b7wD8ZyocgEEIVNDk5FE21Qaxoj3hBPW2SAvb+P39dQ++g5vvcHOhRQieEKCkcL/x+3ioY+ijrbBlTdste3c62sVEnpfoLW16iJlgGDd0mqDvydp+PdikXEZSLb5f5P+XOL6uzAPUWGIe8vQlk7qkYP6jCe5QZ+SiBZminON4Gs0ajcXXDhFAN/nYYZcue5 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, Apr 05, 2023 at 09:58:44PM +0000, Ackerley Tng wrote: > > Thanks again for your review! > > Christian Brauner writes: > > On Tue, Apr 04, 2023 at 03:53:13PM +0200, Christian Brauner wrote: > > > On Fri, Mar 31, 2023 at 11:50:39PM +0000, Ackerley Tng wrote: > > > > > > > > ... > > > > > > > > -SYSCALL_DEFINE1(memfd_restricted, unsigned int, flags) > > > > +static int restrictedmem_create(struct vfsmount *mount) > > > > { > > > > struct file *file, *restricted_file; > > > > int fd, err; > > > > > > > > - if (flags) > > > > - return -EINVAL; > > > > - > > > > fd = get_unused_fd_flags(0); > > > > Any reasons the file descriptors aren't O_CLOEXEC by default? I don't > > > see any reasons why we should introduce new fdtypes that aren't > > > O_CLOEXEC by default. The "don't mix-and-match" train has already left > > > the station anyway as we do have seccomp noitifer fds and pidfds both of > > > which are O_CLOEXEC by default. > > > Thanks for pointing this out. I agree with using O_CLOEXEC, but didn’t > notice this before. Let us discuss this under the original series at > [1]. > > > > > if (fd < 0) > > > > return fd; > > > > > > > > - file = shmem_file_setup("memfd:restrictedmem", 0, VM_NORESERVE); > > > > + if (mount) > > > > + file = shmem_file_setup_with_mnt(mount, "memfd:restrictedmem", > > > 0, VM_NORESERVE); > > > > + else > > > > + file = shmem_file_setup("memfd:restrictedmem", 0, VM_NORESERVE); > > > > + > > > > if (IS_ERR(file)) { > > > > err = PTR_ERR(file); > > > > goto err_fd; > > > > @@ -223,6 +225,66 @@ SYSCALL_DEFINE1(memfd_restricted, unsigned > > > int, flags) > > > > return err; > > > > } > > > > > > > > +static bool is_shmem_mount(struct vfsmount *mnt) > > > > +{ > > > > + return mnt && mnt->mnt_sb && mnt->mnt_sb->s_magic == TMPFS_MAGIC; > > > > This can just be if (mnt->mnt_sb->s_magic == TMPFS_MAGIC). > > > Will simplify this in the next revision. > > > > > +} > > > > + > > > > +static bool is_mount_root(struct file *file) > > > > +{ > > > > + return file->f_path.dentry == file->f_path.mnt->mnt_root; > > > > mount -t tmpfs tmpfs /mnt > > > touch /mnt/bla > > > touch /mnt/ble > > > mount --bind /mnt/bla /mnt/ble > > > fd = open("/mnt/ble") > > > fd_restricted = memfd_restricted(fd) > > > > IOW, this doesn't restrict it to the tmpfs root. It only restricts it to > > > paths that refer to the root of any tmpfs mount. To exclude bind-mounts > > > that aren't bind-mounts of the whole filesystem you want: > > > > path->dentry == path->mnt->mnt_root && > > > path->mnt->mnt_root == path->mnt->mnt_sb->s_root > > > Will adopt this in the next revision and add a selftest to check > this. Thanks for pointing this out! > > > > > +} > > > > + > > > > +static int restrictedmem_create_on_user_mount(int mount_fd) > > > > +{ > > > > + int ret; > > > > + struct fd f; > > > > + struct vfsmount *mnt; > > > > + > > > > + f = fdget_raw(mount_fd); > > > > + if (!f.file) > > > > + return -EBADF; > > > > + > > > > + ret = -EINVAL; > > > > + if (!is_mount_root(f.file)) > > > > + goto out; > > > > + > > > > + mnt = f.file->f_path.mnt; > > > > + if (!is_shmem_mount(mnt)) > > > > + goto out; > > > > + > > > > + ret = file_permission(f.file, MAY_WRITE | MAY_EXEC); > > > > With the current semantics you're asking whether you have write > > > permissions on the /mnt/ble file in order to get answer to the question > > > whether you're allowed to create an unlinked restricted memory file. > > > That doesn't make much sense afaict. > > > That's true. Since mnt_want_write() already checks for write permissions > and this syscall creates an unlinked file on the mount, we don't have to > check permissions on the file then. Will remove this in the next > revision! > > > > > + if (ret) > > > > + goto out; > > > > + > > > > + ret = mnt_want_write(mnt); > > > > + if (unlikely(ret)) > > > > + goto out; > > > > + > > > > + ret = restrictedmem_create(mnt); > > > > + > > > > + mnt_drop_write(mnt); > > > > +out: > > > > + fdput(f); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +SYSCALL_DEFINE2(memfd_restricted, unsigned int, flags, int, mount_fd) > > > > +{ > > > > + if (flags & ~RMFD_USERMNT) > > > > + return -EINVAL; > > > > + > > > > + if (flags == RMFD_USERMNT) { > > > > Why do you even need this flag? It seems that @mount_fd being < 0 is > > > sufficient to indicate that a new restricted memory fd is supposed to be > > > created in the system instance. > > > I'm hoping to have this patch series merged after Chao's patch series > introduces the memfd_restricted() syscall [1]. I'm curious, is there an LSFMM session for this?