Re: [RFC v2 19/19] ima: Setup securityfs for IMA namespace

On Mon, 2021-12-06 at 09:03 -0500, Stefan Berger wrote:

On 12/5/21 23:27, James Bottomley wrote:

quoted

On Fri, 2021-12-03 at 14:11 -0500, Stefan Berger wrote:

quoted

On 12/3/21 13:50, James Bottomley wrote:

quoted

On Fri, 2021-12-03 at 13:06 -0500, Stefan Berger wrote:

[...]

quoted

quoted

quoted

I suppose any late filesystem init callchain would have to be
connected to the user_namespace somehow?

  
I don't think so; I think just moving some securityfs entries
into
the user_namespace and managing the notifier chain from within
securityfs will do for now.  [although I'd have to spec this
out in
code before I knew for sure].

It doesn't have to be right in the user_namespace. The IMA
namespace
is  connected to the user namespace and holds the dentries now...

Please spec it out...

OK, this is what I have.  fill_super turned out to be a locking
nightmare, so I triggered it from free context instead (which
doesn't
have the once per keyed superblock property, so I added a flag in
the
user namespace).  I've got it to the point where the event is
triggered
on mount and unmount, so all the entries for the namespace are
added
when the filesystem is mounted and remove when it's
unmounted.  This
style of addition no longer needs the simple_pin_fs, because the
add/remove callbacks substitute (plus, if we pinned, the free_super
wouldn't trigger on unmount).  The default behaviour still does
pinning
and unpinning, but that can be keyed off the current
user_namespace.

This is all on top of your current series ... some of the functions
should probably be renamed, but I kept them to show how the code
was
migrating in this sketch.

James

---

 From 59c45daa8698c66c3bcebfb194123977d548a9a6 Mon Sep 17 00:00:00
2001
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Sat, 4 Dec 2021 16:38:37 +0000
Subject: [PATCH] rework securityfs

---

-
-static void _securityfs_remove(struct dentry *dentry,
-			       struct vfsmount **mount, int
*mount_count)
+void securityfs_remove(struct dentry *dentry)
  {
  	struct inode *dir;
+	struct user_namespace *ns = current_user_ns();

I had problems with this in this place. So I had to use use

struct user_namespace *user_ns = dentry->d_sb->s_user_ns;

Yes, I think that works ... the owner in the parent namespace could
actually unmount it, so keying off the user namespace it was mounted on
is definitely the correct form.

I'll try to split up your patch and post a v3 with then. Or is it too
early?

It's never too early to see what the series is shaping up as.  However,
I'm still not sure I got the right trigger for the SECURITYFS_NS_ADD
notifier, so that may still have to move ... or even that there isn't
some locking subtlety I missed in triggering SECURITY_NS_REMOVE from
kill_sb.

I also suspect Christian will want a pointer to the securityfs pieces
in struct user_namespace rather than discrete elements added directly.

James