Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <62f946ec160296b6b20bee98986b2bafb8427718.camel@linux.ibm.com>
Subject: Re: [PATCH v10 11/27] ima: Move ima_lsm_policy_notifier into
 ima_namespace
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Stefan Berger <stefanb@linux.ibm.com>,
        linux-integrity@vger.kernel.org
Cc:     serge@hallyn.com, christian.brauner@ubuntu.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, jejb@linux.ibm.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
Date:   Thu, 17 Feb 2022 15:30:53 -0500
In-Reply-To: <20220201203735.164593-12-stefanb@linux.ibm.com>
References: <20220201203735.164593-1-stefanb@linux.ibm.com>
         <20220201203735.164593-12-stefanb@linux.ibm.com>
Content-Type: text/plain; charset="ISO-8859-15"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote:

The builtin IMA policy rules are broad and may be constrained by
loading a custom policy, which could be defined in terms of LSM labels.
When an LSM policy is loaded, existing LSM labels might be affected or
even removed.  In either case, IMA policy rules based on LSM
labels, need to reflect these changes.  If an LSM label is removed,
instead of deleting the IMA policy rule based on the LSM label, the IMA
policy rule is made inactive.

> Move the ima_lsm_policy_notifier into the ima_namespace. Each IMA
> namespace can now register its own LSM policy change notifier callback.
> The policy change notifier for the init_ima_ns still remains in init_ima()
> and therefore handle the registration of the callback for all other
> namespaces in init_ima_namespace().
> 
> Suppress the kernel warning 'rule for LSM <label> is undefined` for all
> other IMA namespaces than the init_ima_ns.

Instead of ignoring the warnings totally, perhaps use either the
"ratelimited" or "once" function options for non init_ima_ns.  It would
be nice if these functions could be namespace aware, so that each
affected IMA namespace would contain at least one warning.

> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> 
> ---
> v10:
>  - Only call pr_warn('rule for LSM <label> is undefined`) for init_ima_ns
> ---
>  security/integrity/ima/ima.h             |  2 ++
>  security/integrity/ima/ima_init_ima_ns.c | 14 ++++++++++++++
>  security/integrity/ima/ima_main.c        |  6 +-----
>  security/integrity/ima/ima_policy.c      | 16 ++++++++++------
>  4 files changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 94c6e3a4d666..fb6bd054d899 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -144,6 +144,8 @@ struct ima_namespace {
>  	int valid_policy;
>  
>  	struct dentry *ima_policy;
> +
> +	struct notifier_block ima_lsm_policy_notifier;
>  } __randomize_layout;
>  extern struct ima_namespace init_ima_ns;
>  
> diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
> index 425eed1c6838..1cba545ae477 100644
> --- a/security/integrity/ima/ima_init_ima_ns.c
> +++ b/security/integrity/ima/ima_init_ima_ns.c
> @@ -10,6 +10,8 @@
>  
>  static int ima_init_namespace(struct ima_namespace *ns)
>  {
> +	int rc;
> +

There's no right or wrong, but the newer IMA code uses 'ret'.

>  	INIT_LIST_HEAD(&ns->ima_default_rules);
>  	INIT_LIST_HEAD(&ns->ima_policy_rules);
>  	INIT_LIST_HEAD(&ns->ima_temp_rules);