On Wed, Jan 6, 2021 at 9:44 PM Lokesh Gidra <lokeshgidra@xxxxxxxxxx> wrote: > On Wed, Jan 6, 2021 at 6:10 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > > On Wed, Nov 11, 2020 at 8:54 PM Lokesh Gidra <lokeshgidra@xxxxxxxxxx> wrote: > > > From: Daniel Colascione <dancol@xxxxxxxxxx> > > > > > > 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@xxxxxxxxxx> > > > > > > [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@xxxxxxxxxx> > > > Reviewed-by: Eric Biggers <ebiggers@xxxxxxxxxx> > > > --- > > > 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
