From: James Bottomley <jejb@linux.ibm.com>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>,
linux-integrity@vger.kernel.org, zohar@linux.ibm.com,
serge@hallyn.com, containers@lists.linux.dev,
dmitry.kasatkin@gmail.com, ebiederm@xmission.com,
krzysztof.struczynski@huawei.com, roberto.sassu@huawei.com,
mpeters@redhat.com, lhinds@redhat.com, lsturman@redhat.com,
puiterwi@redhat.com, jamjoom@us.ibm.com,
linux-kernel@vger.kernel.org, paul@paul-moore.com,
rgb@redhat.com, linux-security-module@vger.kernel.org,
jmorris@namei.org
Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns
Date: Tue, 07 Dec 2021 12:13:02 -0500 [thread overview]
Message-ID: <27f3aecc693bc4a423423416bbc5bf7213b59959.camel@linux.ibm.com> (raw)
In-Reply-To: <20211207145901.awiibdgdidbshsbf@wittgenstein>
On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
[...]
> I would propose not to use the notifier logic. While it might be
> nifty it's over-engineered in my opinion. The dentry stashing in
> struct user_namespace currently serves the purpose to make it
> retrievable in ima_fs_ns_init(). That doesn't justify its existence
> imho.
This is the incremental to Stefan's set with the notifier removed and
the root dentry threaded.
James
---
From d9322270157531f4772fe862fa1655993a0c387d Mon Sep 17 00:00:00 2001
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Mon, 6 Dec 2021 20:27:00 +0000
Subject: [PATCH] Incremental for root dentry
---
include/linux/ima.h | 2 +
include/linux/security.h | 8 ----
include/linux/user_namespace.h | 4 --
security/inode.c | 71 ++++++++++-----------------------
security/integrity/ima/ima_fs.c | 40 ++++---------------
5 files changed, 29 insertions(+), 96 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index cab5fc6caeb3..a6e93bb5ce8a 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -40,6 +40,8 @@ extern int ima_measure_critical_data(const char *event_label,
const char *event_name,
const void *buf, size_t buf_len,
bool hash, u8 *digest, size_t digest_len);
+extern int ima_fs_ns_init(struct user_namespace *ns, struct dentry *root);
+extern void ima_fs_ns_free_dentries(struct user_namespace *ns);
#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
extern void ima_appraise_parse_cmdline(void);
diff --git a/include/linux/security.h b/include/linux/security.h
index a34109c5e3ed..bbf44a466832 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1929,14 +1929,6 @@ struct dentry *securityfs_create_symlink(const char *name,
const struct inode_operations *iops);
extern void securityfs_remove(struct dentry *dentry);
-enum {
- SECURITYFS_NS_ADD,
- SECURITYFS_NS_REMOVE,
-};
-
-extern int securityfs_register_ns_notifier(struct notifier_block *nb);
-extern int securityfs_unregister_ns_notifier(struct notifier_block *nb);
-
#else /* CONFIG_SECURITYFS */
static inline struct dentry *securityfs_create_dir(const char *name,
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 6b8bd060d8c4..5249db04d62b 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -103,10 +103,6 @@ struct user_namespace {
#ifdef CONFIG_IMA
struct ima_namespace *ima_ns;
#endif
-#ifdef CONFIG_SECURITYFS
- struct vfsmount *securityfs_mount;
- bool securityfs_notifier_sent;
-#endif
} __randomize_layout;
struct ucounts {
diff --git a/security/inode.c b/security/inode.c
index 45211845fc31..0b173792fbd3 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -16,18 +16,17 @@
#include <linux/fs_context.h>
#include <linux/mount.h>
#include <linux/pagemap.h>
+#include <linux/ima.h>
#include <linux/init.h>
#include <linux/namei.h>
-#include <linux/notifier.h>
#include <linux/security.h>
#include <linux/lsm_hooks.h>
#include <linux/magic.h>
#include <linux/user_namespace.h>
+static struct vfsmount *securityfs_mount;
static int securityfs_mount_count;
-static BLOCKING_NOTIFIER_HEAD(securityfs_ns_notifier);
-
static void securityfs_free_inode(struct inode *inode)
{
if (S_ISLNK(inode->i_mode))
@@ -40,36 +39,11 @@ static const struct super_operations securityfs_super_operations = {
.free_inode = securityfs_free_inode,
};
-static struct file_system_type fs_type;
-
-static void securityfs_free_context(struct fs_context *fc)
-{
- struct user_namespace *ns = fc->user_ns;
-
- if (ns == &init_user_ns ||
- ns->securityfs_notifier_sent)
- return;
-
- ns->securityfs_notifier_sent = true;
-
- ns->securityfs_mount = vfs_kern_mount(&fs_type, SB_KERNMOUNT,
- fs_type.name, NULL);
- if (IS_ERR(ns->securityfs_mount)) {
- printk(KERN_ERR "kern mount on securityfs ERROR: %ld\n",
- PTR_ERR(ns->securityfs_mount));
- ns->securityfs_mount = NULL;
- return;
- }
-
- blocking_notifier_call_chain(&securityfs_ns_notifier,
- SECURITYFS_NS_ADD, fc->user_ns);
- mntput(ns->securityfs_mount);
-}
-
static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
static const struct tree_descr files[] = {{""}};
int error;
+ struct user_namespace *ns = fc->user_ns;
error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
if (error)
@@ -77,6 +51,10 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
sb->s_op = &securityfs_super_operations;
+ if (ns != &init_user_ns) {
+ ima_fs_ns_init(ns, sb->s_root);
+ }
+
return 0;
}
@@ -87,7 +65,6 @@ static int securityfs_get_tree(struct fs_context *fc)
static const struct fs_context_operations securityfs_context_ops = {
.get_tree = securityfs_get_tree,
- .free = securityfs_free_context,
};
static int securityfs_init_fs_context(struct fs_context *fc)
@@ -100,12 +77,10 @@ static void securityfs_kill_super(struct super_block *sb)
{
struct user_namespace *ns = sb->s_fs_info;
- if (ns != &init_user_ns)
- blocking_notifier_call_chain(&securityfs_ns_notifier,
- SECURITYFS_NS_REMOVE,
- sb->s_fs_info);
- ns->securityfs_notifier_sent = false;
- ns->securityfs_mount = NULL;
+ if (ns != &init_user_ns) {
+ ima_fs_ns_free_dentries(ns);
+ }
+
kill_litter_super(sb);
}
@@ -117,16 +92,6 @@ static struct file_system_type fs_type = {
.fs_flags = FS_USERNS_MOUNT,
};
-int securityfs_register_ns_notifier(struct notifier_block *nb)
-{
- return blocking_notifier_chain_register(&securityfs_ns_notifier, nb);
-}
-
-int securityfs_unregister_ns_notifier(struct notifier_block *nb)
-{
- return blocking_notifier_chain_unregister(&securityfs_ns_notifier, nb);
-}
-
/**
* securityfs_create_dentry - create a dentry in the securityfs filesystem
*
@@ -174,14 +139,18 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
pr_debug("securityfs: creating file '%s'\n",name);
if (ns == &init_user_ns) {
- error = simple_pin_fs(&fs_type, &ns->securityfs_mount,
+ error = simple_pin_fs(&fs_type, &securityfs_mount,
&securityfs_mount_count);
if (error)
return ERR_PTR(error);
}
- if (!parent)
- parent = ns->securityfs_mount->mnt_root;
+ if (!parent) {
+ if (ns == &init_user_ns)
+ parent = securityfs_mount->mnt_root;
+ else
+ return ERR_PTR(-EINVAL);
+ }
dir = d_inode(parent);
@@ -227,7 +196,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
out:
inode_unlock(dir);
if (ns == &init_user_ns)
- simple_release_fs(&ns->securityfs_mount,
+ simple_release_fs(&securityfs_mount,
&securityfs_mount_count);
return dentry;
}
@@ -371,7 +340,7 @@ void securityfs_remove(struct dentry *dentry)
}
inode_unlock(dir);
if (ns == &init_user_ns)
- simple_release_fs(&ns->securityfs_mount,
+ simple_release_fs(&securityfs_mount,
&securityfs_mount_count);
}
EXPORT_SYMBOL_GPL(securityfs_remove);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index c17a6b7eeb95..fb29cb7b0384 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -446,9 +446,10 @@ static const struct file_operations ima_measure_policy_ops = {
.llseek = generic_file_llseek,
};
-static void ima_fs_ns_free_dentries(struct ima_namespace *ns)
+void ima_fs_ns_free_dentries(struct user_namespace *user_ns)
{
int i;
+ struct ima_namespace *ns = user_ns->ima_ns;
for (i = IMAFS_DENTRY_LAST - 1; i >= 0; i--)
securityfs_remove(ns->dentry[i]);
@@ -456,7 +457,7 @@ static void ima_fs_ns_free_dentries(struct ima_namespace *ns)
memset(ns->dentry, 0, sizeof(ns->dentry));
}
-static int ima_fs_ns_init(struct user_namespace *user_ns)
+int ima_fs_ns_init(struct user_namespace *user_ns, struct dentry *root)
{
struct ima_namespace *ns = user_ns->ima_ns;
struct dentry *ima_dir;
@@ -468,7 +469,7 @@ static int ima_fs_ns_init(struct user_namespace *user_ns)
/* FIXME: update when evm and integrity are namespaced */
if (user_ns != &init_user_ns)
ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR] =
- securityfs_create_dir("integrity", NULL);
+ securityfs_create_dir("integrity", root);
else
ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR] = integrity_dir;
@@ -480,7 +481,7 @@ static int ima_fs_ns_init(struct user_namespace *user_ns)
ima_dir = ns->dentry[IMAFS_DENTRY_DIR];
ns->dentry[IMAFS_DENTRY_SYMLINK] =
- securityfs_create_symlink("ima", NULL, "integrity/ima", NULL);
+ securityfs_create_symlink("ima", root, "integrity/ima", NULL);
if (IS_ERR(ns->dentry[IMAFS_DENTRY_SYMLINK]))
goto out;
@@ -520,38 +521,11 @@ static int ima_fs_ns_init(struct user_namespace *user_ns)
return 0;
out:
- ima_fs_ns_free_dentries(ns);
+ ima_fs_ns_free_dentries(user_ns);
return -1;
}
-static int ima_ns_notify(struct notifier_block *this, unsigned long msg,
- void *data)
-{
- int rc = 0;
- struct user_namespace *user_ns = data;
-
- switch (msg) {
- case SECURITYFS_NS_ADD:
- rc = ima_fs_ns_init(user_ns);
- break;
- case SECURITYFS_NS_REMOVE:
- ima_fs_ns_free_dentries(user_ns->ima_ns);
- break;
- }
- return rc;
-}
-
-static struct notifier_block ima_ns_notifier = {
- .notifier_call = ima_ns_notify,
-};
-
int ima_fs_init()
{
- int rc;
-
- rc = securityfs_register_ns_notifier(&ima_ns_notifier);
- if (rc)
- return rc;
-
- return ima_fs_ns_init(&init_user_ns);
+ return ima_fs_ns_init(&init_user_ns, NULL);
}
--
2.33.0
next prev parent reply other threads:[~2021-12-07 17:13 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-06 17:25 [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns Stefan Berger
2021-12-06 17:25 ` [PATCH v3 01/16] ima: Add IMA namespace support Stefan Berger
2021-12-06 17:25 ` [PATCH v3 02/16] ima: Define ns_status for storing namespaced iint data Stefan Berger
2021-12-06 17:25 ` [PATCH v3 03/16] ima: Namespace audit status flags Stefan Berger
2021-12-06 17:25 ` [PATCH v3 04/16] ima: Move delayed work queue and variables into ima_namespace Stefan Berger
2021-12-06 17:25 ` [PATCH v3 05/16] ima: Move IMA's keys queue related " Stefan Berger
2021-12-06 17:25 ` [PATCH v3 06/16] ima: Move policy " Stefan Berger
2021-12-06 17:25 ` [PATCH v3 07/16] ima: Move ima_htable " Stefan Berger
2021-12-06 17:25 ` [PATCH v3 08/16] ima: Move measurement list related variables " Stefan Berger
2021-12-06 17:25 ` [PATCH v3 09/16] ima: Only accept AUDIT rules for IMA non-init_ima_ns namespaces for now Stefan Berger
2021-12-06 17:25 ` [PATCH v3 10/16] ima: Implement hierarchical processing of file accesses Stefan Berger
2021-12-06 17:25 ` [PATCH v3 11/16] securityfs: Move vfsmount into user_namespace Stefan Berger
2021-12-06 17:25 ` [PATCH v3 12/16] securityfs: Extend securityfs with namespacing support Stefan Berger
2021-12-06 17:25 ` [PATCH v3 13/16] ima: Move some IMA policy and filesystem related variables into ima_namespace Stefan Berger
2021-12-06 17:25 ` [PATCH v3 14/16] ima: Use mac_admin_ns_capable() to check corresponding capability Stefan Berger
2021-12-06 17:25 ` [PATCH v3 15/16] ima: Move dentries into ima_namespace Stefan Berger
2021-12-06 17:26 ` [PATCH v3 16/16] ima: Setup securityfs for IMA namespace Stefan Berger
2021-12-06 21:14 ` [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns James Bottomley
2021-12-06 22:13 ` Stefan Berger
2021-12-07 14:59 ` Christian Brauner
2021-12-07 15:16 ` James Bottomley
2021-12-07 15:40 ` James Bottomley
2021-12-07 15:48 ` Casey Schaufler
2021-12-07 17:06 ` James Bottomley
2021-12-07 17:13 ` James Bottomley [this message]
2021-12-07 15:17 ` Christian Brauner
2021-12-07 15:57 ` Stefan Berger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27f3aecc693bc4a423423416bbc5bf7213b59959.camel@linux.ibm.com \
--to=jejb@linux.ibm.com \
--cc=christian.brauner@ubuntu.com \
--cc=containers@lists.linux.dev \
--cc=dmitry.kasatkin@gmail.com \
--cc=ebiederm@xmission.com \
--cc=jamjoom@us.ibm.com \
--cc=jmorris@namei.org \
--cc=krzysztof.struczynski@huawei.com \
--cc=lhinds@redhat.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=lsturman@redhat.com \
--cc=mpeters@redhat.com \
--cc=paul@paul-moore.com \
--cc=puiterwi@redhat.com \
--cc=rgb@redhat.com \
--cc=roberto.sassu@huawei.com \
--cc=serge@hallyn.com \
--cc=stefanb@linux.ibm.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).