lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 6 Jan 2021 22:08:13 -0500 From: Paul Moore <paul@...l-moore.com> To: Lokesh Gidra <lokeshgidra@...gle.com> Cc: Andrea Arcangeli <aarcange@...hat.com>, Alexander Viro <viro@...iv.linux.org.uk>, James Morris <jmorris@...ei.org>, Stephen Smalley <stephen.smalley.work@...il.com>, Casey Schaufler <casey@...aufler-ca.com>, Eric Biggers <ebiggers@...nel.org>, "Serge E. Hallyn" <serge@...lyn.com>, Eric Paris <eparis@...isplace.org>, Daniel Colascione <dancol@...col.org>, Kees Cook <keescook@...omium.org>, "Eric W. Biederman" <ebiederm@...ssion.com>, KP Singh <kpsingh@...gle.com>, David Howells <dhowells@...hat.com>, Anders Roxell <anders.roxell@...aro.org>, Sami Tolvanen <samitolvanen@...gle.com>, Matthew Garrett <matthewgarrett@...gle.com>, Aaron Goidel <acgoide@...ho.nsa.gov>, Randy Dunlap <rdunlap@...radead.org>, "Joel Fernandes (Google)" <joel@...lfernandes.org>, YueHaibing <yuehaibing@...wei.com>, Christian Brauner <christian.brauner@...ntu.com>, Alexei Starovoitov <ast@...nel.org>, Alexey Budankov <alexey.budankov@...ux.intel.com>, Adrian Reber <areber@...hat.com>, Aleksa Sarai <cyphar@...har.com>, Linux FS Devel <linux-fsdevel@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>, LSM List <linux-security-module@...r.kernel.org>, SElinux list <selinux@...r.kernel.org>, Kalesh Singh <kaleshsingh@...gle.com>, Calin Juravle <calin@...gle.com>, Suren Baghdasaryan <surenb@...gle.com>, Jeffrey Vander Stoep <jeffv@...gle.com>, "Cc: Android Kernel" <kernel-team@...roid.com>, "open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>, Andrew Morton <akpm@...ux-foundation.org>, hch@...radead.org, Daniel Colascione <dancol@...gle.com>, Eric Biggers <ebiggers@...gle.com> Subject: Re: [PATCH v13 2/4] fs: add LSM-supporting anon-inode interface On Wed, Jan 6, 2021 at 9:44 PM Lokesh Gidra <lokeshgidra@...gle.com> wrote: > On Wed, Jan 6, 2021 at 6:10 PM Paul Moore <paul@...l-moore.com> wrote: > > > > On Wed, Nov 11, 2020 at 8:54 PM Lokesh Gidra <lokeshgidra@...gle.com> wrote: > > > From: Daniel Colascione <dancol@...gle.com> > > > > > > This change adds a new function, anon_inode_getfd_secure, that creates > > > anonymous-node file with individual non-S_PRIVATE inode to which security > > > modules can apply policy. Existing callers continue using the original > > > singleton-inode kind of anonymous-inode file. We can transition anonymous > > > inode users to the new kind of anonymous inode in individual patches for > > > the sake of bisection and review. > > > > > > The new function accepts an optional context_inode parameter that callers > > > can use to provide additional contextual information to security modules. > > > For example, in case of userfaultfd, the created inode is a 'logical child' > > > of the context_inode (userfaultfd inode of the parent process) in the sense > > > that it provides the security context required during creation of the child > > > process' userfaultfd inode. > > > > > > Signed-off-by: Daniel Colascione <dancol@...gle.com> > > > > > > [Delete obsolete comments to alloc_anon_inode()] > > > [Add context_inode description in comments to anon_inode_getfd_secure()] > > > [Remove definition of anon_inode_getfile_secure() as there are no callers] > > > [Make __anon_inode_getfile() static] > > > [Use correct error cast in __anon_inode_getfile()] > > > [Fix error handling in __anon_inode_getfile()] > > > > Lokesh, I'm assuming you made the changes in the brackets above? If > > so they should include your initials or some other means of > > attributing them to you, e.g. "[LG: Fix error ...]". > > Thanks for reviewing the patch. Sorry for missing this. If it's > critical then I can upload another version of the patches to fix this. > Kindly let me know. Normally that is something I could fix during a merge with your approval, but see my comments to patch 3/4; I think this patchset still needs some work. > > > Signed-off-by: Lokesh Gidra <lokeshgidra@...gle.com> > > > Reviewed-by: Eric Biggers <ebiggers@...gle.com> > > > --- > > > fs/anon_inodes.c | 150 ++++++++++++++++++++++++++---------- > > > fs/libfs.c | 5 -- > > > include/linux/anon_inodes.h | 5 ++ > > > 3 files changed, 115 insertions(+), 45 deletions(-) ... > > > +static struct file *__anon_inode_getfile(const char *name, > > > + const struct file_operations *fops, > > > + void *priv, int flags, > > > + const struct inode *context_inode, > > > + bool secure) > > > > Is it necessary to pass both the context_inode pointer and the secure > > boolean? It seems like if context_inode is non-NULL then one could > > assume that a secure anonymous inode was requested; is there ever > > going to be a case where this is not true? > > Yes, it is necessary as there are scenarios where a secure anon-inode > is to be created but there is no context_inode available. For > instance, in patch 4/4 of this series you'll see that when a secure > anon-inode is created in the userfaultfd syscall, context_inode isn't > available. My mistake, I didn't realize this until I got further in the patchset. -- paul moore www.paul-moore.com
Powered by blists - more mailing lists