On Wed, Jan 6, 2021 at 9:44 PM Lokesh Gidra [off-list ref] wrote:

On Wed, Jan 6, 2021 at 6:10 PM Paul Moore [off-list ref] wrote:

quoted

On Wed, Nov 11, 2020 at 8:54 PM Lokesh Gidra [off-list ref] wrote:

quoted

From: Daniel Colascione <redacted>

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 <redacted>

[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.

quoted

quoted

Signed-off-by: Lokesh Gidra <redacted>
Reviewed-by: Eric Biggers <redacted>
---
 fs/anon_inodes.c            | 150 ++++++++++++++++++++++++++----------
 fs/libfs.c                  |   5 --
 include/linux/anon_inodes.h |   5 ++
 3 files changed, 115 insertions(+), 45 deletions(-)

...

quoted

quoted

+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