Thread (19 messages) 19 messages, 4 authors, 2021-01-08

Re: [PATCH v13 3/4] selinux: teach SELinux about anonymous inodes

From: Lokesh Gidra <hidden>
Date: 2021-01-08 21:32:31
Also in: linux-fsdevel, linux-mm, lkml, selinux

On Fri, Jan 8, 2021 at 1:24 PM Stephen Smalley
[off-list ref] wrote:
On Fri, Jan 8, 2021 at 3:17 PM Lokesh Gidra [off-list ref] wrote:
quoted
On Fri, Jan 8, 2021 at 11:35 AM Stephen Smalley
[off-list ref] wrote:
quoted
On Wed, Jan 6, 2021 at 10:03 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 uses the anon_inodes and LSM infrastructure introduced in
the previous patches to give SELinux the ability to control
anonymous-inode files that are created using the new
anon_inode_getfd_secure() function.

A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".

Example:

type uffd_t;
type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:anon_inode { create };

(The next patch in this series is necessary for making userfaultfd
support this new interface.  The example above is just
for exposition.)

Signed-off-by: Daniel Colascione <redacted>
Signed-off-by: Lokesh Gidra <redacted>
---
 security/selinux/hooks.c            | 56 +++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 ++
 2 files changed, 58 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 6b1826fc3658..d092aa512868 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2927,6 +2927,61 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
        return 0;
 }

+static int selinux_inode_init_security_anon(struct inode *inode,
+                                           const struct qstr *name,
+                                           const struct inode *context_inode)
+{
+       const struct task_security_struct *tsec = selinux_cred(current_cred());
+       struct common_audit_data ad;
+       struct inode_security_struct *isec;
+       int rc;
+
+       if (unlikely(!selinux_initialized(&selinux_state)))
+               return 0;
+
+       isec = selinux_inode(inode);
+
+       /*
+        * We only get here once per ephemeral inode.  The inode has
+        * been initialized via inode_alloc_security but is otherwise
+        * untouched.
+        */
+
+       if (context_inode) {
+               struct inode_security_struct *context_isec =
+                       selinux_inode(context_inode);
+               if (context_isec->initialized != LABEL_INITIALIZED)
+                       return -EACCES;
Stephen, as per your explanation below, is this check also
problematic? I mean is it possible that /dev/kvm context_inode may not
have its label initialized? If so, then v12 of the patch series can be
used as is. Otherwise, I will send the next version which rollbacks
v14 and v13, except for this check. Kindly confirm.
The context_inode should always be initialized already.  I'm not fond
though of silently returning -EACCES here.  At the least we should
have a pr_err() or pr_warn() here.  In reality, this could only occur
in the case of a kernel bug or memory corruption so it used to be a
candidate for WARN_ON() or BUG_ON() or similar but I know that
BUG_ON() at least is frowned upon these days.
Got it. I'll add a pr_err(). Thanks a lot.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help