[PATCH v5 0/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[PATCH v5 0/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[GUO Zihua]

IMA happens to measure extra files if LSM based rules are specified and
the corresponding LSM is updating its policy.

The root cause is explained in the second patch.

GUO Zihua (2):
  ima: Simplify ima_lsm_copy_rule
  ima: Handle -ESTALE returned by ima_filter_rule_match()

 security/integrity/ima/ima_policy.c | 51 ++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 16 deletions(-)
---

v5:
  Updated code to avoid reusing rule. Fixed a potential mem leak caused
by race condition. Updated commit message for the first patch based on
Mimi's feedback.

v4:
  Use a tempory rule instead of updating the rule in place. To do that,
also update ima_lsm_copy_rule so we can make use of it.

v3:
  Update current rule instead of just retrying, as suggested by Mimi

v2:
  Fixes message errors pointed out by Mimi

-- 
2.17.1

[PATCH v5 1/2] ima: Simplify ima_lsm_copy_rule

[GUO Zihua]

Currently ima_lsm_copy_rule() set the arg_p field of the source rule to
NULL, so that the source rule could be freed afterward. It does not make
sense for this behavior to be inside a "copy" function. So move it
outside and let the caller handle this field.

ima_lsm_copy_rule() now produce a shallow copy of the original entry
including args_p field. Meaning only the lsm.rule and the rule itself
should be freed for the original rule. Thus, instead of calling
ima_lsm_free_rule() which frees lsm.rule as well as args_p field, free
the lsm.rule directly.

Signed-off-by: GUO Zihua <guozihua@huawei.com>
---
 security/integrity/ima/ima_policy.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a8802b8da946..8040215c0252 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -398,12 +398,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
 
 		nentry->lsm[i].type = entry->lsm[i].type;
 		nentry->lsm[i].args_p = entry->lsm[i].args_p;
-		/*
-		 * Remove the reference from entry so that the associated
-		 * memory will not be freed during a later call to
-		 * ima_lsm_free_rule(entry).
-		 */
-		entry->lsm[i].args_p = NULL;
 
 		ima_filter_rule_init(nentry->lsm[i].type, Audit_equal,
 				     nentry->lsm[i].args_p,
@@ -417,6 +411,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
 
 static int ima_lsm_update_rule(struct ima_rule_entry *entry)
 {
+	int i;
 	struct ima_rule_entry *nentry;
 
 	nentry = ima_lsm_copy_rule(entry);
@@ -431,7 +426,8 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry)
 	 * references and the entry itself. All other memory references will now
 	 * be owned by nentry.
 	 */
-	ima_lsm_free_rule(entry);
+	for (i = 0; i < MAX_LSM_RULES; i++)
+		ima_filter_rule_free(entry->lsm[i].rule);
 	kfree(entry);
 
 	return 0;
-- 
2.17.1

[PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[GUO Zihua]

IMA relies on the blocking LSM policy notifier callback to update the
LSM based IMA policy rules.

When SELinux update its policies, IMA would be notified and starts
updating all its lsm rules one-by-one. During this time, -ESTALE would
be returned by ima_filter_rule_match() if it is called with a LSM rule
that has not yet been updated. In ima_match_rules(), -ESTALE is not
handled, and the LSM rule is considered a match, causing extra files
to be measured by IMA.

Fix it by re-initializing a temporary rule if -ESTALE is returned by
ima_filter_rule_match(). The origin rule in the rule list would be
updated by the LSM policy notifier callback.

Fixes: b16942455193 ("ima: use the lsm policy update notifier")
Signed-off-by: GUO Zihua <guozihua@huawei.com>
---
 security/integrity/ima/ima_policy.c | 41 ++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 8040215c0252..2edff7f58c25 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -545,6 +545,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
 			    const char *func_data)
 {
 	int i;
+	bool result = false;
+	struct ima_rule_entry *lsm_rule = rule;
+	bool rule_reinitialized = false;
 
 	if ((rule->flags & IMA_FUNC) &&
 	    (rule->func != func && func != POST_SETATTR))
@@ -606,35 +609,55 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
 		int rc = 0;
 		u32 osid;
 
-		if (!rule->lsm[i].rule) {
-			if (!rule->lsm[i].args_p)
+		if (!lsm_rule->lsm[i].rule) {
+			if (!lsm_rule->lsm[i].args_p)
 				continue;
 			else
 				return false;
 		}
+
+retry:
 		switch (i) {
 		case LSM_OBJ_USER:
 		case LSM_OBJ_ROLE:
 		case LSM_OBJ_TYPE:
 			security_inode_getsecid(inode, &osid);
-			rc = ima_filter_rule_match(osid, rule->lsm[i].type,
+			rc = ima_filter_rule_match(osid, lsm_rule->lsm[i].type,
 						   Audit_equal,
-						   rule->lsm[i].rule);
+						   lsm_rule->lsm[i].rule);
 			break;
 		case LSM_SUBJ_USER:
 		case LSM_SUBJ_ROLE:
 		case LSM_SUBJ_TYPE:
-			rc = ima_filter_rule_match(secid, rule->lsm[i].type,
+			rc = ima_filter_rule_match(secid, lsm_rule->lsm[i].type,
 						   Audit_equal,
-						   rule->lsm[i].rule);
+						   lsm_rule->lsm[i].rule);
 			break;
 		default:
 			break;
 		}
-		if (!rc)
-			return false;
+
+		if (rc == -ESTALE && !rule_reinitialized) {
+			lsm_rule = ima_lsm_copy_rule(rule);
+			if (lsm_rule) {
+				rule_reinitialized = true;
+				goto retry;
+			}
+		}
+		if (!rc) {
+			result = false;
+			goto out;
+		}
 	}
-	return true;
+	result = true;
+
+out:
+	if (rule_reinitialized) {
+		for (i = 0; i < MAX_LSM_RULES; i++)
+			ima_filter_rule_free(lsm_rule->lsm[i].rule);
+		kfree(lsm_rule);
+	}
+	return result;
 }
 
 /*
-- 
2.17.1

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Mimi Zohar]

Hi Scott,

On Wed, 2022-09-21 at 20:58 +0800, GUO Zihua wrote:
>                 }
> -               if (!rc)
> -                       return false;
> +
> +               if (rc == -ESTALE && !rule_reinitialized) {

Ok, this limits allocating ima_lsm_copy_rule() to the first -ESTALE,

> +                       lsm_rule = ima_lsm_copy_rule(rule);
> +                       if (lsm_rule) {
> +                               rule_reinitialized = true;
> +                               goto retry;

but "retry" is also limited to the first -ESTALE.

> +                       }
> +               }
> +               if (!rc) {
> +                       result = false;
> +                       goto out;
> +               }
>         }
> -       return true;
> +       result = true;
> +
> +out:
> +       if (rule_reinitialized) {
> +               for (i = 0; i < MAX_LSM_RULES; i++)
> +                       ima_filter_rule_free(lsm_rule->lsm[i].rule);
> +               kfree(lsm_rule);
> +       }
> +       return result;
>  }

-- 

thanks,

Mimi

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

On 2022/9/22 19:09, Mimi Zohar wrote:
> Hi Scott,
> 
> On Wed, 2022-09-21 at 20:58 +0800, GUO Zihua wrote:
>>                  }
>> -               if (!rc)
>> -                       return false;
>> +
>> +               if (rc == -ESTALE && !rule_reinitialized) {
> 
> Ok, this limits allocating ima_lsm_copy_rule() to the first -ESTALE,
> 
>> +                       lsm_rule = ima_lsm_copy_rule(rule);
>> +                       if (lsm_rule) {
>> +                               rule_reinitialized = true;
>> +                               goto retry;
> 
> but "retry" is also limited to the first -ESTALE.

Technically we would only need one retry. This loop is looping on all 
the lsm members of one rule, and ima_lsm_copy_rule would update all the 
lsm members of this rule. The "lsm member" here refers to LSM defined 
properties like obj_user, obj_role etc. These members are of AND 
relation, meaning all lsm members together would form one LSM rule.

As of the scenario you mentioned, I think it should be really rare. 
Spending to much time and code on this might not worth it.
> 
>> +                       }
>> +               }
>> +               if (!rc) {
>> +                       result = false;
>> +                       goto out;
>> +               }
>>          }
>> -       return true;
>> +       result = true;
>> +
>> +out:
>> +       if (rule_reinitialized) {
>> +               for (i = 0; i < MAX_LSM_RULES; i++)
>> +                       ima_filter_rule_free(lsm_rule->lsm[i].rule);
>> +               kfree(lsm_rule);
>> +       }
>> +       return result;
>>   }
> 


-- 
Best
GUO Zihua

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Mimi Zohar]

On Fri, 2022-09-23 at 12:01 +0800, Guozihua (Scott) wrote:
> On 2022/9/22 19:09, Mimi Zohar wrote:
> > Hi Scott,
> > 
> > On Wed, 2022-09-21 at 20:58 +0800, GUO Zihua wrote:
> >>                  }
> >> -               if (!rc)
> >> -                       return false;
> >> +
> >> +               if (rc == -ESTALE && !rule_reinitialized) {
> > 
> > Ok, this limits allocating ima_lsm_copy_rule() to the first -ESTALE,
> > 
> >> +                       lsm_rule = ima_lsm_copy_rule(rule);
> >> +                       if (lsm_rule) {
> >> +                               rule_reinitialized = true;
> >> +                               goto retry;
> > 
> > but "retry" is also limited to the first -ESTALE.
> 
> Technically we would only need one retry. This loop is looping on all 
> the lsm members of one rule, and ima_lsm_copy_rule would update all the 
> lsm members of this rule. The "lsm member" here refers to LSM defined 
> properties like obj_user, obj_role etc. These members are of AND 
> relation, meaning all lsm members together would form one LSM rule.
> 
> As of the scenario you mentioned, I think it should be really rare. 
> Spending to much time and code on this might not worth it.
> > 
> >> +                       }
> >> +               }

Either there can be multiple LSM fields and the memory is allocated
once and freed once at the end, as you suggested, or the memory should
be freed here and rule_reinitialized reset, minimizing the code change.

> >> +               if (!rc) {
> >> +                       result = false;
> >> +                       goto out;
> >> +               }
> >>          }
> >> -       return true;
> >> +       result = true;
> >> +
> >> +out:
> >> +       if (rule_reinitialized) {
> >> +               for (i = 0; i < MAX_LSM_RULES; i++)
> >> +                       ima_filter_rule_free(lsm_rule->lsm[i].rule);
> >> +               kfree(lsm_rule);
> >> +       }
> >> +       return result;
> >>   }

-- 
thanks,

Mimi

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

On 2022/9/23 19:19, Mimi Zohar wrote:
> On Fri, 2022-09-23 at 12:01 +0800, Guozihua (Scott) wrote:
>> On 2022/9/22 19:09, Mimi Zohar wrote:
>>> Hi Scott,
>>>
>>> On Wed, 2022-09-21 at 20:58 +0800, GUO Zihua wrote:
>>>>                   }
>>>> -               if (!rc)
>>>> -                       return false;
>>>> +
>>>> +               if (rc == -ESTALE && !rule_reinitialized) {
>>>
>>> Ok, this limits allocating ima_lsm_copy_rule() to the first -ESTALE,
>>>
>>>> +                       lsm_rule = ima_lsm_copy_rule(rule);
>>>> +                       if (lsm_rule) {
>>>> +                               rule_reinitialized = true;
>>>> +                               goto retry;
>>>
>>> but "retry" is also limited to the first -ESTALE.
>>
>> Technically we would only need one retry. This loop is looping on all
>> the lsm members of one rule, and ima_lsm_copy_rule would update all the
>> lsm members of this rule. The "lsm member" here refers to LSM defined
>> properties like obj_user, obj_role etc. These members are of AND
>> relation, meaning all lsm members together would form one LSM rule.
>>
>> As of the scenario you mentioned, I think it should be really rare.
>> Spending to much time and code on this might not worth it.
>>>
>>>> +                       }
>>>> +               }
> 
> Either there can be multiple LSM fields and the memory is allocated
> once and freed once at the end, as you suggested, or the memory should
> be freed here and rule_reinitialized reset, minimizing the code change.

I might have overlooked something, but if I understands the code 
correctly, we would be copying the same rule over and over again till 
the loop ends in that case. ima_lsm_update_rule() would replace the rule 
node on the rule list without updating the rule in place. Although 
synchronize_rcu() should prevent a UAF, the rule in ima_match_rules() 
would not be updated. Meaning SELinux would always return -ESTALE before 
we copy and retry as we keep passing in the outdated rule entry.
> 
>>>> +               if (!rc) {
>>>> +                       result = false;
>>>> +                       goto out;
>>>> +               }
>>>>           }
>>>> -       return true;
>>>> +       result = true;
>>>> +
>>>> +out:
>>>> +       if (rule_reinitialized) {
>>>> +               for (i = 0; i < MAX_LSM_RULES; i++)
>>>> +                       ima_filter_rule_free(lsm_rule->lsm[i].rule);
>>>> +               kfree(lsm_rule);
>>>> +       }
>>>> +       return result;
>>>>    }
> 


-- 
Best
GUO Zihua

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Mimi Zohar]

On Sat, 2022-09-24 at 14:05 +0800, Guozihua (Scott) wrote:

> I might have overlooked something, but if I understands the code 
> correctly, we would be copying the same rule over and over again till 
> the loop ends in that case. ima_lsm_update_rule() would replace the rule 
> node on the rule list without updating the rule in place. Although 
> synchronize_rcu() should prevent a UAF, the rule in ima_match_rules() 
> would not be updated. Meaning SELinux would always return -ESTALE before 
> we copy and retry as we keep passing in the outdated rule entry.

After reviewing this patch set again, the code looks fine.  The commit
message is still a bit off, but I've pushed the patch set out to next-
integrity-testing, waiting for some Reviewed-by/Tested-by tags.

-- 
thanks,

Mimi

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Roberto Sassu]

On Wed, 2022-09-28 at 10:11 -0400, Mimi Zohar wrote:
> On Sat, 2022-09-24 at 14:05 +0800, Guozihua (Scott) wrote:
> 
> > I might have overlooked something, but if I understands the code 
> > correctly, we would be copying the same rule over and over again
> > till 
> > the loop ends in that case. ima_lsm_update_rule() would replace the
> > rule 
> > node on the rule list without updating the rule in place. Although 
> > synchronize_rcu() should prevent a UAF, the rule in
> > ima_match_rules() 
> > would not be updated. Meaning SELinux would always return -ESTALE
> > before 
> > we copy and retry as we keep passing in the outdated rule entry.
> 
> After reviewing this patch set again, the code looks fine.  The
> commit
> message is still a bit off, but I've pushed the patch set out to
> next-
> integrity-testing, waiting for some Reviewed-by/Tested-by tags.

The patches look ok for me. For both:

Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

Roberto

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

On 2022/9/28 22:11, Mimi Zohar wrote:
> 
> After reviewing this patch set again, the code looks fine.  The commit
> message is still a bit off, but I've pushed the patch set out to next-
> integrity-testing, waiting for some Reviewed-by/Tested-by tags.
> 

Hi Mimi,

How's this patch going? I see Roberto is replying with a Reviewed-by.

-- 
Best
GUO Zihua

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Mimi Zohar]

On Tue, 2022-10-18 at 16:43 +0800, Guozihua (Scott) wrote:
> On 2022/9/28 22:11, Mimi Zohar wrote:
> > 
> > After reviewing this patch set again, the code looks fine.  The commit
> > message is still a bit off, but I've pushed the patch set out to next-
> > integrity-testing, waiting for some Reviewed-by/Tested-by tags.
> > 
> 
> Hi Mimi,
> 
> How's this patch going? I see Roberto is replying with a Reviewed-by.

I'd really like to see a "Tested-by" tag as well.

Are you able to force the scenario?

-- 
thanks,

Mimi

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

On 2022/10/19 9:07, Mimi Zohar wrote:
> On Tue, 2022-10-18 at 16:43 +0800, Guozihua (Scott) wrote:
>> On 2022/9/28 22:11, Mimi Zohar wrote:
>>>
>>> After reviewing this patch set again, the code looks fine.  The commit
>>> message is still a bit off, but I've pushed the patch set out to next-
>>> integrity-testing, waiting for some Reviewed-by/Tested-by tags.
>>>
>>
>> Hi Mimi,
>>
>> How's this patch going? I see Roberto is replying with a Reviewed-by.
> 
> I'd really like to see a "Tested-by" tag as well.
> 
> Are you able to force the scenario?
> 

It's a race condition which could be hard to reproduce easily and in a 
stable manner. I'll give it a try.
-- 
Best
GUO Zihua

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

On 2022/10/19 15:17, Guozihua (Scott) wrote:
> On 2022/10/19 9:07, Mimi Zohar wrote:
>> On Tue, 2022-10-18 at 16:43 +0800, Guozihua (Scott) wrote:
>>> On 2022/9/28 22:11, Mimi Zohar wrote:
>>>>
>>>> After reviewing this patch set again, the code looks fine.  The commit
>>>> message is still a bit off, but I've pushed the patch set out to next-
>>>> integrity-testing, waiting for some Reviewed-by/Tested-by tags.
>>>>
>>>
>>> Hi Mimi,
>>>
>>> How's this patch going? I see Roberto is replying with a Reviewed-by.
>>
>> I'd really like to see a "Tested-by" tag as well.
>>
>> Are you able to force the scenario?
>>
> 
> It's a race condition which could be hard to reproduce easily and in a 
> stable manner. I'll give it a try.

Hi Mimi,

I managed to re-produce this issue with the help of the following two 
scripts:

read_tmp_measurement.sh:
> #!/bin/bash
> 
> while true
> do
>         cat /root/tmp.txt > /dev/null
>         measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp\.txt" | wc -l`
>         if [ "${measurement}" == "1" ]; then
>                 echo "measurement found"
>                 exit 1
>         fi
> done

test.sh:
> #!/bin/bash
> 
> echo "measure obj_user=system_u obj_role=object_r obj_type=unlabeled_t" > /sys/kernel/security/ima/policy
> 
> cat /root/tmp2.txt
> measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp2\.txt" | wc -l`
> [ "$measurement" == "1" ] && echo "measurement for tmp2 found"
> 
> cat /root/tmp.txt
> measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp\.txt" | wc -l`
> [ "$measurement" == "1" ] && echo "measurement for tmp found, preparation failed!" && exit 1
> 
> ./read_tmp_measurement.sh &
> pid=$!
> 
> cd /usr/share/selinux/default
> semodule -i clock.pp.bz2
> semodule -r clock
> 
> kill ${pid}

I created two files tmp.txt and tmp2.txt, assign them with type 
user_home_t and unlabeled_t respectively and then run test.sh.
On a multi-core environment, I managed to reproduce this issue pretty 
easily and tested that once the solution is merged, the issue stops 
happening.

-- 
Best
GUO Zihua

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Mimi Zohar]

Hi Scott,

On Fri, 2022-10-28 at 16:36 +0800, Guozihua (Scott) wrote:
> 
> I managed to re-produce this issue with the help of the following two 
> scripts:
> 
> read_tmp_measurement.sh:
> > #!/bin/bash
> > 
> > while true
> > do
> >         cat /root/tmp.txt > /dev/null
> >         measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp\.txt" | wc -l`
> >         if [ "${measurement}" == "1" ]; then
> >                 echo "measurement found"
> >                 exit 1
> >         fi
> > done
> 
> test.sh:
> > #!/bin/bash
> > 
> > echo "measure obj_user=system_u obj_role=object_r obj_type=unlabeled_t" > /sys/kernel/security/ima/policy
> > 
> > cat /root/tmp2.txt
> > measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp2\.txt" | wc -l`
> > [ "$measurement" == "1" ] && echo "measurement for tmp2 found"
> > 
> > cat /root/tmp.txt
> > measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp\.txt" | wc -l`
> > [ "$measurement" == "1" ] && echo "measurement for tmp found, preparation failed!" && exit 1
> > 
> > ./read_tmp_measurement.sh &
> > pid=$!
> > 
> > cd /usr/share/selinux/default
> > semodule -i clock.pp.bz2
> > semodule -r clock
> > 
> > kill ${pid}

Are you loading/unloading any selinux policy or specifically clock? If
specifically clock, what is special about it?

> I created two files tmp.txt and tmp2.txt, assign them with type 
> user_home_t and unlabeled_t respectively and then run test.sh.
> On a multi-core environment, I managed to reproduce this issue pretty 
> easily and tested that once the solution is merged, the issue stops 
> happening.

As I only see an IMA measurement policy rule being loaded for
"unlabeled_t" and not "user_home_t", should I assume that an IMA
measurement rule already exists for "user_home_t"?

thanks,

Mimi

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

Hi Mimi,

On 2022/11/2 6:15, Mimi Zohar wrote:
> Hi Scott,
> 
> On Fri, 2022-10-28 at 16:36 +0800, Guozihua (Scott) wrote:
>>
>> I managed to re-produce this issue with the help of the following two
>> scripts:
>>
>> read_tmp_measurement.sh:
>>> #!/bin/bash
>>>
>>> while true
>>> do
>>>          cat /root/tmp.txt > /dev/null
>>>          measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp\.txt" | wc -l`
>>>          if [ "${measurement}" == "1" ]; then
>>>                  echo "measurement found"
>>>                  exit 1
>>>          fi
>>> done
>>
>> test.sh:
>>> #!/bin/bash
>>>
>>> echo "measure obj_user=system_u obj_role=object_r obj_type=unlabeled_t" > /sys/kernel/security/ima/policy
>>>
>>> cat /root/tmp2.txt
>>> measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp2\.txt" | wc -l`
>>> [ "$measurement" == "1" ] && echo "measurement for tmp2 found"
>>>
>>> cat /root/tmp.txt
>>> measurement=`cat /sys/kernel/security/ima/ascii_runtime_measurements | grep "tmp\.txt" | wc -l`
>>> [ "$measurement" == "1" ] && echo "measurement for tmp found, preparation failed!" && exit 1
>>>
>>> ./read_tmp_measurement.sh &
>>> pid=$!
>>>
>>> cd /usr/share/selinux/default
>>> semodule -i clock.pp.bz2
>>> semodule -r clock
>>>
>>> kill ${pid}
> 
> Are you loading/unloading any selinux policy or specifically clock? If
> specifically clock, what is special about it?

No there are nothing special about clock. Any selinux policy should do,.
> 
>> I created two files tmp.txt and tmp2.txt, assign them with type
>> user_home_t and unlabeled_t respectively and then run test.sh.
>> On a multi-core environment, I managed to reproduce this issue pretty
>> easily and tested that once the solution is merged, the issue stops
>> happening.
> 
> As I only see an IMA measurement policy rule being loaded for
> "unlabeled_t" and not "user_home_t", should I assume that an IMA
> measurement rule already exists for "user_home_t"?

There wasn't a rule for user_home_t. These scripts demonstrate that 
during a selinux policy reload, IMA would measure files that is not in 
the range of it's LSM based rules. Which is the issue I am trying to fix.

In this test, we only have one rule for measuring files of type 
unlabeled_t. However, during selinux policy reload, file of user_home_t 
is also measured.
> 
> thanks,
> 
> Mimi
> 

-- 
Best
GUO Zihua

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Mimi Zohar]

On Wed, 2022-11-02 at 09:42 +0800, Guozihua (Scott) wrote:
> > As I only see an IMA measurement policy rule being loaded for
> > "unlabeled_t" and not "user_home_t", should I assume that an IMA
> > measurement rule already exists for "user_home_t"?
> 
> There wasn't a rule for user_home_t. These scripts demonstrate that 
> during a selinux policy reload, IMA would measure files that is not in 
> the range of it's LSM based rules. Which is the issue I am trying to fix.
> 
> In this test, we only have one rule for measuring files of type 
> unlabeled_t. However, during selinux policy reload, file of user_home_t 
> is also measured.

Thanks, Scott.  After tweaking the scripts for my system, I was able to
reproduce the bug.  This patch set is now queued in next-integrity.

-- 
thanks, 

Mimi

Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match()

[Guozihua (Scott)]

On 2022/11/3 21:15, Mimi Zohar wrote:
> On Wed, 2022-11-02 at 09:42 +0800, Guozihua (Scott) wrote:
>>> As I only see an IMA measurement policy rule being loaded for
>>> "unlabeled_t" and not "user_home_t", should I assume that an IMA
>>> measurement rule already exists for "user_home_t"?
>>
>> There wasn't a rule for user_home_t. These scripts demonstrate that
>> during a selinux policy reload, IMA would measure files that is not in
>> the range of it's LSM based rules. Which is the issue I am trying to fix.
>>
>> In this test, we only have one rule for measuring files of type
>> unlabeled_t. However, during selinux policy reload, file of user_home_t
>> is also measured.
> 
> Thanks, Scott.  After tweaking the scripts for my system, I was able to
> reproduce the bug.  This patch set is now queued in next-integrity.
> 

Hi Mimi,

Any chance these patches would be in 6.1?
-- 
Best
GUO Zihua