* [PATCH v5 0/2] ima: Handle -ESTALE returned by ima_filter_rule_match() @ 2022-09-21 12:58 GUO Zihua 2022-09-21 12:58 ` [PATCH v5 1/2] ima: Simplify ima_lsm_copy_rule GUO Zihua 2022-09-21 12:58 ` [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua 0 siblings, 2 replies; 17+ messages in thread From: GUO Zihua @ 2022-09-21 12:58 UTC (permalink / raw) To: zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v5 1/2] ima: Simplify ima_lsm_copy_rule 2022-09-21 12:58 [PATCH v5 0/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua @ 2022-09-21 12:58 ` GUO Zihua 2022-09-21 12:58 ` [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua 1 sibling, 0 replies; 17+ messages in thread From: GUO Zihua @ 2022-09-21 12:58 UTC (permalink / raw) To: zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-21 12:58 [PATCH v5 0/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua 2022-09-21 12:58 ` [PATCH v5 1/2] ima: Simplify ima_lsm_copy_rule GUO Zihua @ 2022-09-21 12:58 ` GUO Zihua 2022-09-22 11:09 ` Mimi Zohar 1 sibling, 1 reply; 17+ messages in thread From: GUO Zihua @ 2022-09-21 12:58 UTC (permalink / raw) To: zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-21 12:58 ` [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua @ 2022-09-22 11:09 ` Mimi Zohar 2022-09-23 4:01 ` Guozihua (Scott) 0 siblings, 1 reply; 17+ messages in thread From: Mimi Zohar @ 2022-09-22 11:09 UTC (permalink / raw) To: GUO Zihua, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-22 11:09 ` Mimi Zohar @ 2022-09-23 4:01 ` Guozihua (Scott) 2022-09-23 11:19 ` Mimi Zohar 0 siblings, 1 reply; 17+ messages in thread From: Guozihua (Scott) @ 2022-09-23 4:01 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-23 4:01 ` Guozihua (Scott) @ 2022-09-23 11:19 ` Mimi Zohar 2022-09-24 6:05 ` Guozihua (Scott) 0 siblings, 1 reply; 17+ messages in thread From: Mimi Zohar @ 2022-09-23 11:19 UTC (permalink / raw) To: Guozihua (Scott), dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-23 11:19 ` Mimi Zohar @ 2022-09-24 6:05 ` Guozihua (Scott) 2022-09-28 14:11 ` Mimi Zohar 0 siblings, 1 reply; 17+ messages in thread From: Guozihua (Scott) @ 2022-09-24 6:05 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-24 6:05 ` Guozihua (Scott) @ 2022-09-28 14:11 ` Mimi Zohar 2022-10-04 14:19 ` Roberto Sassu 2022-10-18 8:43 ` Guozihua (Scott) 0 siblings, 2 replies; 17+ messages in thread From: Mimi Zohar @ 2022-09-28 14:11 UTC (permalink / raw) To: Guozihua (Scott), dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-28 14:11 ` Mimi Zohar @ 2022-10-04 14:19 ` Roberto Sassu 2022-10-18 8:43 ` Guozihua (Scott) 1 sibling, 0 replies; 17+ messages in thread From: Roberto Sassu @ 2022-10-04 14:19 UTC (permalink / raw) To: Mimi Zohar, Guozihua (Scott), dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-09-28 14:11 ` Mimi Zohar 2022-10-04 14:19 ` Roberto Sassu @ 2022-10-18 8:43 ` Guozihua (Scott) 2022-10-19 1:07 ` Mimi Zohar 1 sibling, 1 reply; 17+ messages in thread From: Guozihua (Scott) @ 2022-10-18 8:43 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-10-18 8:43 ` Guozihua (Scott) @ 2022-10-19 1:07 ` Mimi Zohar 2022-10-19 7:17 ` Guozihua (Scott) 0 siblings, 1 reply; 17+ messages in thread From: Mimi Zohar @ 2022-10-19 1:07 UTC (permalink / raw) To: Guozihua (Scott), dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-10-19 1:07 ` Mimi Zohar @ 2022-10-19 7:17 ` Guozihua (Scott) 2022-10-28 8:36 ` Guozihua (Scott) 0 siblings, 1 reply; 17+ messages in thread From: Guozihua (Scott) @ 2022-10-19 7:17 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-10-19 7:17 ` Guozihua (Scott) @ 2022-10-28 8:36 ` Guozihua (Scott) 2022-11-01 22:15 ` Mimi Zohar 0 siblings, 1 reply; 17+ messages in thread From: Guozihua (Scott) @ 2022-10-28 8:36 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-10-28 8:36 ` Guozihua (Scott) @ 2022-11-01 22:15 ` Mimi Zohar 2022-11-02 1:42 ` Guozihua (Scott) 0 siblings, 1 reply; 17+ messages in thread From: Mimi Zohar @ 2022-11-01 22:15 UTC (permalink / raw) To: Guozihua (Scott), dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-11-01 22:15 ` Mimi Zohar @ 2022-11-02 1:42 ` Guozihua (Scott) 2022-11-03 13:15 ` Mimi Zohar 0 siblings, 1 reply; 17+ messages in thread From: Guozihua (Scott) @ 2022-11-02 1:42 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-11-02 1:42 ` Guozihua (Scott) @ 2022-11-03 13:15 ` Mimi Zohar 2022-11-14 3:31 ` Guozihua (Scott) 0 siblings, 1 reply; 17+ messages in thread From: Mimi Zohar @ 2022-11-03 13:15 UTC (permalink / raw) To: Guozihua (Scott), dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() 2022-11-03 13:15 ` Mimi Zohar @ 2022-11-14 3:31 ` Guozihua (Scott) 0 siblings, 0 replies; 17+ messages in thread From: Guozihua (Scott) @ 2022-11-14 3:31 UTC (permalink / raw) To: Mimi Zohar, dmitry.kasatkin, paul, jmorris, serge Cc: linux-integrity, linux-security-module, Janne Karhunen, Roberto Sassu 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-11-14 3:31 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-21 12:58 [PATCH v5 0/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua 2022-09-21 12:58 ` [PATCH v5 1/2] ima: Simplify ima_lsm_copy_rule GUO Zihua 2022-09-21 12:58 ` [PATCH v5 2/2] ima: Handle -ESTALE returned by ima_filter_rule_match() GUO Zihua 2022-09-22 11:09 ` Mimi Zohar 2022-09-23 4:01 ` Guozihua (Scott) 2022-09-23 11:19 ` Mimi Zohar 2022-09-24 6:05 ` Guozihua (Scott) 2022-09-28 14:11 ` Mimi Zohar 2022-10-04 14:19 ` Roberto Sassu 2022-10-18 8:43 ` Guozihua (Scott) 2022-10-19 1:07 ` Mimi Zohar 2022-10-19 7:17 ` Guozihua (Scott) 2022-10-28 8:36 ` Guozihua (Scott) 2022-11-01 22:15 ` Mimi Zohar 2022-11-02 1:42 ` Guozihua (Scott) 2022-11-03 13:15 ` Mimi Zohar 2022-11-14 3:31 ` Guozihua (Scott)
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 NNTP newsgroup(s).