Hi Tejun,

On Wed, Jan 30, 2019 at 6:09 PM Tejun Heo [off-list ref] wrote:

Hello,

On Wed, Jan 30, 2019 at 12:41:50PM +0100, Ondrej Mosnacek wrote:

quoted

@@ -673,6 +698,12 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
                      goto err_out3;
      }

+     if (parent) {
+             ret = kernfs_node_init_security(parent, kn);
+             if (ret)
+                     goto err_out3;
+     }

So, doing this unconditionally isn't a good idea.  kernfs doesn't use
the usual dentry/inode because there are machines with 6, even 7 digit
number of kernfs nodes and some of them even failed to boot due to
memory shortage.  Please don't blow it up by default.

Hm, I see... basically the only thing that gets allocated in
kernfs_node_init_security() by default (at least under SELinux/ no
LSM) is the kernfs_iattrs structures, so I assume you are pointing at
that. I think this can be easily fixed, if we again use the assumption
that whenever the parent node has only default attributes
(parent->iattrs == NULL), then the child node should also have just
default attributes (and so we don't need to call kernfs_iattrs() on it
nor call the security hook). Something along these lines:

[...]
+static int kernfs_node_init_security(struct kernfs_node *parent,
+                                    struct kernfs_node *kn)
+{
+       struct kernfs_iattrs *attrs, *pattrs;
+       struct qstr q;
+
+       pattrs = parent->iattrs;
+       if (!pattrs)
+               return 0;
+
+       attrs = kernfs_iattrs(kn);
+       if (!attrs)
+               return -ENOMEM;
+
+       q.name = kn->name;
+       q.hash_len = hashlen_string(parent, kn->name);
[...]

Technically this might make some LSMs unhappy, if they want to set
some non-default context even if parent is all default, but this is
already impossible now and in this case I think we have no better
choice than sacrificing a bit of flexibility for memory efficiency,
which is apparently critical here.

Tejun, Casey, would the above modification be fine with you?

--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.