Hi Stefan,
On Tue, 2022-01-04 at 12:04 -0500, Stefan Berger wrote:
> From: Stefan Berger <stefanb@xxxxxxxxxxxxx>
>
> Move variables related to the IMA policy into the ima_namespace. This way
> the IMA policy of an IMA namespace can be set and displayed using a
> front-end like SecurityFS.
>
> Implement ima_ns_from_file() to get the IMA namespace via the user
> namespace of the SecurityFS superblock that a file belongs to.
>
> To get the current ima_namespace use get_current_ns() when a function
> that is related to a policy rule is called. In other cases where functions
> are called due file attribute modifications, use init_ima_ns, since these
> functions are related to IMA appraisal and changes to file attributes are
> only relevant to the init_ima_ns until IMA namespaces also support IMA
> appraisal. In ima_file_free() use init_ima_ns since in this case flags
> related to file measurements may be affected, which is not supported in
> IMA namespaces, yet.
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Please split this patch into "ima: pass through ima namespace", or some
other name, and "ima: Move policy related variables into
ima_namespace". The other option is to combine the "pass through ima
namespace" with the 2nd patch, like Christian's example.
> ---
> security/integrity/ima/ima.h | 49 ++++---
> security/integrity/ima/ima_api.c | 8 +-
> security/integrity/ima/ima_appraise.c | 28 ++--
> security/integrity/ima/ima_asymmetric_keys.c | 4 +-
> security/integrity/ima/ima_fs.c | 16 ++-
> security/integrity/ima/ima_init.c | 8 +-
> security/integrity/ima/ima_init_ima_ns.c | 6 +
> security/integrity/ima/ima_main.c | 83 +++++++----
> security/integrity/ima/ima_policy.c | 142 ++++++++++---------
> security/integrity/ima/ima_queue_keys.c | 11 +-
> 10 files changed, 213 insertions(+), 142 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index c4af3275f015..0b3dc9425076 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -20,6 +20,7 @@
> #include <linux/hash.h>
> #include <linux/tpm.h>
> #include <linux/audit.h>
> +#include <linux/user_namespace.h>
> #include <crypto/hash_info.h>
>
> #include "../integrity.h"
> @@ -43,9 +44,6 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>
> #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
>
> -/* current content of the policy */
> -extern int ima_policy_flag;
> -
> /* bitset of digests algorithms allowed in the setxattr hook */
> extern atomic_t ima_setxattr_allowed_hash_algorithms;
>
> @@ -120,6 +118,14 @@ struct ima_kexec_hdr {
> };
>
> struct ima_namespace {
> + struct list_head ima_default_rules;
> + /* ns's policy rules */
Thank you for adding comments. Why is the ima_default_rules not
considered "ns's policy rules"? Will this come later or is it limited
to init_ima_ns?
> + struct list_head ima_policy_rules;
> + struct list_head ima_temp_rules;
> + /* Pointer to ns's current policy */
> + struct list_head __rcu *ima_rules;
Since "Pointer to ns's current policy" only refers to ima_rules, append
it to the variable definition.
> + /* current content of the policy */
> + int ima_policy_flag;
Similarly here append the comment to the variable definition.
> } __randomize_layout;
> extern struct ima_namespace init_ima_ns;
thanks,
Mimi
