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 A8F96C4167D for ; Mon, 6 Nov 2023 11:42:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0C7776B020C; Mon, 6 Nov 2023 06:42:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0781D6B020D; Mon, 6 Nov 2023 06:42:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E81B16B020E; Mon, 6 Nov 2023 06:42:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D8AAA6B020C for ; Mon, 6 Nov 2023 06:42:12 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id AA9F4C0782 for ; Mon, 6 Nov 2023 11:42:12 +0000 (UTC) X-FDA: 81427340904.24.23231D7 Received: from mail-qv1-f50.google.com (mail-qv1-f50.google.com [209.85.219.50]) by imf10.hostedemail.com (Postfix) with ESMTP id C0F98C0005 for ; Mon, 6 Nov 2023 11:42:10 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=qyiZKrQr; spf=pass (imf10.hostedemail.com: domain of tabba@google.com designates 209.85.219.50 as permitted sender) smtp.mailfrom=tabba@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1699270930; 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=sHi6HNfMoiiRWrk//ou0On9wauTiypiZpuQxJoOASgE=; b=wyEsXAnJi/XOkmiv4dFl8uYR9amCRLzaOuS6Gxi/uhvF0zk19fN97XGzdafPX8QCX14Ktl C14oNDXOIyl6a4XwcgcCBcH/ByLZQof1DDKetlnwf8WPlxJ0C7uZv7njqqDcn6OdM73Iw8 Rwco7T4IjuO+LJcOqM700UAJoWTnpuI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699270930; a=rsa-sha256; cv=none; b=VR+rta3KadKSs5XGakzwFZQSDC27qYlmqd+oR5gFlKVraMWLDWU9mKaM9nb34AKlBBFJQX jJMOAbpSSmDaWIs5aM+wBELkLMTXMW2Dy3QkHvG+tMolc28BuOumcWCHa61afwXWXaxHIi 8sjCPSx8UlivwH0+YIUTaZyOOvAc3xU= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=qyiZKrQr; spf=pass (imf10.hostedemail.com: domain of tabba@google.com designates 209.85.219.50 as permitted sender) smtp.mailfrom=tabba@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qv1-f50.google.com with SMTP id 6a1803df08f44-66d134a019cso31151986d6.3 for ; Mon, 06 Nov 2023 03:42:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1699270930; x=1699875730; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=sHi6HNfMoiiRWrk//ou0On9wauTiypiZpuQxJoOASgE=; b=qyiZKrQrU7OO8gaTfVEooYHIJJcI3Xnp2+lRcRnFGqPEutbx5/NcTez6ON+y2AZGnt 4TXV1l4KXnEAhbS6RG/yKtvi9pS0snFYBfeqBydTXgIelX3AOd8Hs3caBHOHk/vLGnU+ avH3TJ7fOR7ZQ6urxd5ijoc+8h5CvkuQyad4Yl80QRzcj3XUAbh/taxXqrKU2Giv0wGG dCTtnrhBxdX0KzJqd9vfBKWXXT2f3gLelBW4SBWaUyylm6qOPq8fjtPXCbWQmdHqiTwf 4/bDTLWZYscSGxiqgPaqJIvfOjIC98NdP7w/iG04aQdWVa9zydmGPjdFRj9+bVgg1adV zVew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699270930; x=1699875730; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=sHi6HNfMoiiRWrk//ou0On9wauTiypiZpuQxJoOASgE=; b=mjKneNa1a1mnQOIcCGOMkykSo+fxN+hkVCgbpEQNKH2QHcc8JcWB4aqAeo9IX5E1nO DwUA/1vRXS9laUQGE5zRq7gfC9cc8Rs3c5FkZtWZmTSwqfE6ab0LSZBAELBUYi1jK49d XsjyIQU6ZPzDSPFKwXRTNF1XPUc/w9CZ4RbEklkiT0poN2rqjPC7fXu7WqfsAF1gD49R BGHH0G/AkIqtOkX423FFL2HB+kFeEvkQ5xYQt8DsMiYfMa2DXK8cndDJsWboYlCXIdnt l6DVQhxD2mAGKhclLLUHIOoBPlvyhlAKnu7BzKVNFqqkgcL+M4twpeRu0N9tYuljEAY1 pgEA== X-Gm-Message-State: AOJu0YzzE+w2iwMXJfKJfW01t3S/gPFVb4U+lcGxKkXWUvuZczXDCvrQ n2dVwOzfqLz6GnV3WHr6NLwmxX+54W6DDUBo0ptw3Q== X-Google-Smtp-Source: AGHT+IFqtL+i8IMlADOUnxNtS9BqYz4IVdy1FTYYXRfD2pg+wEXN2AL/1eIHCBiP2/6Ge2uIUEuwBkXjW7tgEM1U/80= X-Received: by 2002:a05:6214:dcf:b0:672:549c:15e8 with SMTP id 15-20020a0562140dcf00b00672549c15e8mr32468533qvt.55.1699270929722; Mon, 06 Nov 2023 03:42:09 -0800 (PST) MIME-Version: 1.0 References: <20231105163040.14904-1-pbonzini@redhat.com> <20231105163040.14904-15-pbonzini@redhat.com> In-Reply-To: <20231105163040.14904-15-pbonzini@redhat.com> From: Fuad Tabba Date: Mon, 6 Nov 2023 11:41:33 +0000 Message-ID: Subject: Re: [PATCH 14/34] fs: Rename anon_inode_getfile_secure() and anon_inode_getfd_secure() To: Paolo Bonzini Cc: Marc Zyngier , Oliver Upton , Huacai Chen , Michael Ellerman , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Sean Christopherson , Alexander Viro , Christian Brauner , "Matthew Wilcox (Oracle)" , Andrew Morton , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Xiaoyao Li , Xu Yilun , Chao Peng , Jarkko Sakkinen , Anish Moorthy , David Matlack , Yu Zhang , Isaku Yamahata , =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= , Vlastimil Babka , Vishal Annapurve , Ackerley Tng , Maciej Szmigiero , David Hildenbrand , Quentin Perret , Michael Roth , Wang , Liam Merwick , Isaku Yamahata , "Kirill A. Shutemov" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 68wnmuu5d7gtesxa8u7frkt7rn6oogkt X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: C0F98C0005 X-Rspam-User: X-HE-Tag: 1699270930-485716 X-HE-Meta: U2FsdGVkX1+sYqRaRZdi+YmIrjwd/705spl+mw2ncDLeA+UZFEb4cG8kRN5Gr8q9chX8iqwvIEahs/m1EQ5XJdqw3NZH+RwLSMKw/2KwlmEEiDaFDVJ+BMF9h61mhbm+uMQwxzQMMhPoVsoI+AzrcMIdL8JMIZ6bGLKHddgRBiSCsFCH3ibui0A/F5cRXgUfCzABk8DS1SklLQZGtQNlUM/UqRxA1q6BEtw9VOEGUlCOhlFgsTmFrtZ/eCSv5RmZX48zVKiBDJ+Jb09NhTQ6IEGA/B/s/qi5hBxi7eFzEZRMPPLD/75UPdvIN+npsg3YEY0ISDewfl0L8XfwHXEkD2ScDBvwQWbDt37dxFd8w58DHpQMaMNrcucYCKmzPtnlQs/OSZVE2SzSjcQdQan/xELJVl8HXM8QPvKBNlXskW3yLMQh4N4AecR9PfAi9EbdGvM+5Ulek2YZq5dmiMvUPxoNoFAnE9x68/X9ehD5sM52OBXbb+sge4xrJGzyVssxl8S0060PzPVa9rv/mvd5n7iddCvswjWuHVi9uCaAHDzZuhTiOe4adB7SwBNt6lVC4248OI+xtU9jHQ2hRPg5Tq3xUQatSrUmn6w5pAZpNHu6Ww8+l2tAlCx2/ITRaIs56o+QTiOX+bFo2uKdVfHAx7rYiSSUezRBhqLwp+/6+2o0Yv/tM/j9i33N8xL37a0DJblB05w8V7fmJtptiEzclQNyFaDyy83ahwLRj9QchH+9Y+5A8JVl0bweACBBeOxc6aY8uzMF6HJAst9dqX7Asa3WSFnpCydhX1nHcBftAfWQ4QBeO8u17f4H7QHjmD0+mOf/SxxmaU8ohqRvqv80CwEp33CmcLGQkItmklUIwR3xFctlX9YD5bquZdlsyppVMGrxLtXhCuIvnAhZX6ho94EYca/2M0jLpiykS+OhW46zLzfKoOKiUXKADIgDHMafOHj3mqqOJGPlcrMKBKN 2GBp85h3 +klEtSgpwp12Q9u6P22VRazE2pGsIAN8ofi9vUGOCUttlyodn0np5bVhkrncItg/q9zMMgidC8WRFW3GAxbQbXxTV4mzdZg89Fd+wUdrmyw9P/Oqkx+OEtVXr0IJSVIllh4z+c8eK55xNeiB0l330D2wHocDqxcN4IcxgpBow6UDXVKw+Kp6tzYgyRuwvOs+yYnk202OIH6bziH59uRH2gntL3EHmOYZaaGpRhAMKZnE4XExAtlMiu0TFCXNG9eoMujhLs5cdWtabDSd5Op5XwFUH3JGgXcYrnTf/10+u3989ujD4CFwnICLMmpQJXV7Buo87OgE4SrUoOLfhxEJwYkNVFf4V94QrQQQyosMrkLRz7e2owGYP1nWYChCDguF5gKePm2soiolzmfoqfG3cqh6+5+WMStWjftrkvjRlIgYBYNm2GDewvIAcWxCNz/IsP44MmhDLnwqj9Oo= 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: List-Subscribe: List-Unsubscribe: On Sun, Nov 5, 2023 at 4:32=E2=80=AFPM Paolo Bonzini = wrote: > > The call to the inode_init_security_anon() LSM hook is not the sole > reason to use anon_inode_getfile_secure() or anon_inode_getfd_secure(). > For example, the functions also allow one to create a file with non-zero > size, without needing a full-blown filesystem. In this case, you don't > need a "secure" version, just unique inodes; the current name of the > functions is confusing and does not explain well the difference with > the more "standard" anon_inode_getfile() and anon_inode_getfd(). > > Of course, there is another side of the coin; neither io_uring nor > userfaultfd strictly speaking need distinct inodes, and it is not > that clear anymore that anon_inode_create_get{file,fd}() allow the LSM > to intercept and block the inode's creation. If one was so inclined, > anon_inode_getfile_secure() and anon_inode_getfd_secure() could be kept, > using the shared inode or a new one depending on CONFIG_SECURITY. > However, this is probably overkill, and potentially a cause of bugs in > different configurations. Therefore, just add a comment to io_uring > and userfaultfd explaining the choice of the function. > > While at it, remove the export for what is now anon_inode_create_getfd(). > There is no in-tree module that uses it, and the old name is gone anyway. > If anybody actually needs the symbol, they can ask or they can just use > anon_inode_create_getfile(), which will be exported very soon for use > in KVM. > > Suggested-by: Christian Brauner > Signed-off-by: Paolo Bonzini > --- Reviewed-by: Fuad Tabba Tested-by: Fuad Tabba Cheers, /fuad > fs/anon_inodes.c | 46 +++++++++++++++++++++++-------------- > fs/userfaultfd.c | 5 ++-- > include/linux/anon_inodes.h | 4 ++-- > io_uring/io_uring.c | 3 ++- > 4 files changed, 36 insertions(+), 22 deletions(-) > > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c > index 24192a7667ed..3d4a27f8b4fe 100644 > --- a/fs/anon_inodes.c > +++ b/fs/anon_inodes.c > @@ -79,7 +79,7 @@ static struct file *__anon_inode_getfile(const char *na= me, > const struct file_operations *fo= ps, > void *priv, int flags, > const struct inode *context_inod= e, > - bool secure) > + bool make_inode) > { > struct inode *inode; > struct file *file; > @@ -87,7 +87,7 @@ static struct file *__anon_inode_getfile(const char *na= me, > if (fops->owner && !try_module_get(fops->owner)) > return ERR_PTR(-ENOENT); > > - if (secure) { > + if (make_inode) { > inode =3D anon_inode_make_secure_inode(name, context_inod= e); > if (IS_ERR(inode)) { > file =3D ERR_CAST(inode); > @@ -149,13 +149,10 @@ struct file *anon_inode_getfile(const char *name, > EXPORT_SYMBOL_GPL(anon_inode_getfile); > > /** > - * anon_inode_getfile_secure - Like anon_inode_getfile(), but creates a = new > + * anon_inode_create_getfile - Like anon_inode_getfile(), but creates a = new > * !S_PRIVATE anon inode rather than reuse t= he > * singleton anon inode and calls the > - * inode_init_security_anon() LSM hook. Thi= s > - * allows for both the inode to have its own > - * security context and for the LSM to enfor= ce > - * policy on the inode's creation. > + * inode_init_security_anon() LSM hook. > * > * @name: [in] name of the "class" of the new file > * @fops: [in] file operations for the new file > @@ -164,11 +161,19 @@ EXPORT_SYMBOL_GPL(anon_inode_getfile); > * @context_inode: > * [in] the logical relationship with the new inode (option= al) > * > + * Create a new anonymous inode and file pair. This can be done for two > + * reasons: > + * - for the inode to have its own security context, so that LSMs can en= force > + * policy on the inode's creation; > + * - if the caller needs a unique inode, for example in order to customi= ze > + * the size returned by fstat() > + * > * The LSM may use @context_inode in inode_init_security_anon(), but a > - * reference to it is not held. Returns the newly created file* or an e= rror > - * pointer. See the anon_inode_getfile() documentation for more informa= tion. > + * reference to it is not held. > + * > + * Returns the newly created file* or an error pointer. > */ > -struct file *anon_inode_getfile_secure(const char *name, > +struct file *anon_inode_create_getfile(const char *name, > const struct file_operations *fops= , > void *priv, int flags, > const struct inode *context_inode) > @@ -181,7 +186,7 @@ static int __anon_inode_getfd(const char *name, > const struct file_operations *fops, > void *priv, int flags, > const struct inode *context_inode, > - bool secure) > + bool make_inode) > { > int error, fd; > struct file *file; > @@ -192,7 +197,7 @@ static int __anon_inode_getfd(const char *name, > fd =3D error; > > file =3D __anon_inode_getfile(name, fops, priv, flags, context_in= ode, > - secure); > + make_inode); > if (IS_ERR(file)) { > error =3D PTR_ERR(file); > goto err_put_unused_fd; > @@ -231,10 +236,9 @@ int anon_inode_getfd(const char *name, const struct = file_operations *fops, > EXPORT_SYMBOL_GPL(anon_inode_getfd); > > /** > - * anon_inode_getfd_secure - Like anon_inode_getfd(), but creates a new > + * anon_inode_create_getfd - Like anon_inode_getfd(), but creates a new > * !S_PRIVATE anon inode rather than reuse the singleton anon inode, and= calls > - * the inode_init_security_anon() LSM hook. This allows the inode to hav= e its > - * own security context and for a LSM to reject creation of the inode. > + * the inode_init_security_anon() LSM hook. > * > * @name: [in] name of the "class" of the new file > * @fops: [in] file operations for the new file > @@ -243,16 +247,24 @@ EXPORT_SYMBOL_GPL(anon_inode_getfd); > * @context_inode: > * [in] the logical relationship with the new inode (option= al) > * > + * Create a new anonymous inode and file pair. This can be done for two > + * reasons: > + * - for the inode to have its own security context, so that LSMs can en= force > + * policy on the inode's creation; > + * - if the caller needs a unique inode, for example in order to customi= ze > + * the size returned by fstat() > + * > * The LSM may use @context_inode in inode_init_security_anon(), but a > * reference to it is not held. > + * > + * Returns a newly created file descriptor or an error code. > */ > -int anon_inode_getfd_secure(const char *name, const struct file_operatio= ns *fops, > +int anon_inode_create_getfd(const char *name, const struct file_operatio= ns *fops, > void *priv, int flags, > const struct inode *context_inode) > { > return __anon_inode_getfd(name, fops, priv, flags, context_inode,= true); > } > -EXPORT_SYMBOL_GPL(anon_inode_getfd_secure); > > static int __init anon_inode_init(void) > { > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 56eaae9dac1a..7a1cf8bab5eb 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1033,7 +1033,7 @@ static int resolve_userfault_fork(struct userfaultf= d_ctx *new, > { > int fd; > > - fd =3D anon_inode_getfd_secure("[userfaultfd]", &userfaultfd_fops= , new, > + fd =3D anon_inode_create_getfd("[userfaultfd]", &userfaultfd_fops= , new, > O_RDONLY | (new->flags & UFFD_SHARED_FCNTL_FLAGS)= , inode); > if (fd < 0) > return fd; > @@ -2205,7 +2205,8 @@ static int new_userfaultfd(int flags) > /* prevent the mm struct to be freed */ > mmgrab(ctx->mm); > > - fd =3D anon_inode_getfd_secure("[userfaultfd]", &userfaultfd_fops= , ctx, > + /* Create a new inode so that the LSM can block the creation. */ > + fd =3D anon_inode_create_getfd("[userfaultfd]", &userfaultfd_fops= , ctx, > O_RDONLY | (flags & UFFD_SHARED_FCNTL_FLAGS), NUL= L); > if (fd < 0) { > mmdrop(ctx->mm); > diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h > index 5deaddbd7927..93a5f16d03f3 100644 > --- a/include/linux/anon_inodes.h > +++ b/include/linux/anon_inodes.h > @@ -15,13 +15,13 @@ struct inode; > struct file *anon_inode_getfile(const char *name, > const struct file_operations *fops, > void *priv, int flags); > -struct file *anon_inode_getfile_secure(const char *name, > +struct file *anon_inode_create_getfile(const char *name, > const struct file_operations *fops= , > void *priv, int flags, > const struct inode *context_inode)= ; > int anon_inode_getfd(const char *name, const struct file_operations *fop= s, > void *priv, int flags); > -int anon_inode_getfd_secure(const char *name, > +int anon_inode_create_getfd(const char *name, > const struct file_operations *fops, > void *priv, int flags, > const struct inode *context_inode); > diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c > index 8d1bc6cdfe71..22b98f47bb28 100644 > --- a/io_uring/io_uring.c > +++ b/io_uring/io_uring.c > @@ -3835,7 +3835,8 @@ static struct file *io_uring_get_file(struct io_rin= g_ctx *ctx) > return ERR_PTR(ret); > #endif > > - file =3D anon_inode_getfile_secure("[io_uring]", &io_uring_fops, = ctx, > + /* Create a new inode so that the LSM can block the creation. */ > + file =3D anon_inode_create_getfile("[io_uring]", &io_uring_fops, = ctx, > O_RDWR | O_CLOEXEC, NULL); > #if defined(CONFIG_UNIX) > if (IS_ERR(file)) { > -- > 2.39.1 > >