On Wed, Apr 20, 2022 at 10:06:14AM -0400, Stefan Berger wrote:
> Move the variables ima_write_mutex, ima_fs_flag, and valid_policy, which
> are related to updating the IMA policy, into the ima_namespace. This way
> each IMA namespace can set these variables independently in its instance
> of securityfs.
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
> Acked-by: Christian Brauner <brauner@xxxxxxxxxx>
> Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
Acked-by: Serge Hallyn <serge@xxxxxxxxxx>
> ---
> security/integrity/ima/ima.h | 5 ++++
> security/integrity/ima/ima_fs.c | 32 +++++++++++-------------
> security/integrity/ima/ima_init_ima_ns.c | 4 +++
> 3 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 82e8af2bf698..a144edfdb9a1 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -137,6 +137,11 @@ struct ima_namespace {
> struct ima_h_table ima_htable;
> struct list_head ima_measurements; /* list of all measurements */
> unsigned long binary_runtime_size; /* used by init_ima_ns */
> +
> + /* securityfs support related variables */
> + struct mutex ima_write_mutex;
> + unsigned long ima_fs_flags;
> + int valid_policy;
> } __randomize_layout;
> extern struct ima_namespace init_ima_ns;
>
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 5ef0e2b2cf64..4cf786f0bba8 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -24,8 +24,6 @@
>
> #include "ima.h"
>
> -static DEFINE_MUTEX(ima_write_mutex);
> -
> bool ima_canonical_fmt;
> static int __init default_canonical_fmt_setup(char *str)
> {
> @@ -36,8 +34,6 @@ static int __init default_canonical_fmt_setup(char *str)
> }
> __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>
> -static int valid_policy = 1;
> -
> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> loff_t *ppos, atomic_long_t *val)
> {
> @@ -338,7 +334,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
> goto out;
> }
>
> - result = mutex_lock_interruptible(&ima_write_mutex);
> + result = mutex_lock_interruptible(&ns->ima_write_mutex);
> if (result < 0)
> goto out_free;
>
> @@ -353,12 +349,12 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
> } else {
> result = ima_parse_add_rule(ns, data);
> }
> - mutex_unlock(&ima_write_mutex);
> + mutex_unlock(&ns->ima_write_mutex);
> out_free:
> kfree(data);
> out:
> if (result < 0)
> - valid_policy = 0;
> + ns->valid_policy = 0;
>
> return result;
> }
> @@ -375,8 +371,6 @@ enum ima_fs_flags {
> IMA_FS_BUSY,
> };
>
> -static unsigned long ima_fs_flags;
> -
> #ifdef CONFIG_IMA_READ_POLICY
> static const struct seq_operations ima_policy_seqops = {
> .start = ima_policy_start,
> @@ -391,6 +385,8 @@ static const struct seq_operations ima_policy_seqops = {
> */
> static int ima_open_policy(struct inode *inode, struct file *filp)
> {
> + struct ima_namespace *ns = &init_ima_ns;
> +
> if (!(filp->f_flags & O_WRONLY)) {
> #ifndef CONFIG_IMA_READ_POLICY
> return -EACCES;
> @@ -402,7 +398,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
> return seq_open(filp, &ima_policy_seqops);
> #endif
> }
> - if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
> + if (test_and_set_bit(IMA_FS_BUSY, &ns->ima_fs_flags))
> return -EBUSY;
> return 0;
> }
> @@ -416,25 +412,25 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
> */
> static int ima_release_policy(struct inode *inode, struct file *file)
> {
> - const char *cause = valid_policy ? "completed" : "failed";
> struct ima_namespace *ns = &init_ima_ns;
> + const char *cause = ns->valid_policy ? "completed" : "failed";
>
> if ((file->f_flags & O_ACCMODE) == O_RDONLY)
> return seq_release(inode, file);
>
> - if (valid_policy && ima_check_policy(ns) < 0) {
> + if (ns->valid_policy && ima_check_policy(ns) < 0) {
> cause = "failed";
> - valid_policy = 0;
> + ns->valid_policy = 0;
> }
>
> pr_info("policy update %s\n", cause);
> integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
> - "policy_update", cause, !valid_policy, 0);
> + "policy_update", cause, !ns->valid_policy, 0);
>
> - if (!valid_policy) {
> + if (!ns->valid_policy) {
> ima_delete_rules(ns);
> - valid_policy = 1;
> - clear_bit(IMA_FS_BUSY, &ima_fs_flags);
> + ns->valid_policy = 1;
> + clear_bit(IMA_FS_BUSY, &ns->ima_fs_flags);
> return 0;
> }
>
> @@ -443,7 +439,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
> securityfs_remove(ima_policy);
> ima_policy = NULL;
> #elif defined(CONFIG_IMA_WRITE_POLICY)
> - clear_bit(IMA_FS_BUSY, &ima_fs_flags);
> + clear_bit(IMA_FS_BUSY, &ns->ima_fs_flags);
> #elif defined(CONFIG_IMA_READ_POLICY)
> inode->i_mode &= ~S_IWUSR;
> #endif
> diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
> index a7477072c587..425eed1c6838 100644
> --- a/security/integrity/ima/ima_init_ima_ns.c
> +++ b/security/integrity/ima/ima_init_ima_ns.c
> @@ -26,6 +26,10 @@ static int ima_init_namespace(struct ima_namespace *ns)
> else
> ns->binary_runtime_size = ULONG_MAX;
>
> + mutex_init(&ns->ima_write_mutex);
> + ns->valid_policy = 1;
> + ns->ima_fs_flags = 0;
> +
> return 0;
> }
>
> --
> 2.34.1
