[Cc'ing Matthew Garret based on the additional bprm call to
process_measurement() - commit d906c10d8a31 ("IMA: Support using new
creds in appraisal policy")]
On Tue, 2019-12-24 at 15:18 -0800, Casey Schaufler wrote:
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index f19a895ad7cd..193ddd55420b 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> for (i = 0; i < MAX_LSM_RULES; i++) {
> int rc = 0;
> u32 osid;
> + struct lsmblob blob;
>
> if (!rule->lsm[i].rule)
> continue;
> @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> case LSM_OBJ_ROLE:
> case LSM_OBJ_TYPE:
> security_inode_getsecid(inode, &osid);
> - rc = security_filter_rule_match(osid,
> + lsmblob_init(&blob, osid);
> + rc = security_filter_rule_match(&blob,
> rule->lsm[i].type,
> Audit_equal,
> rule->lsm[i].rule);
> @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> case LSM_SUBJ_USER:
> case LSM_SUBJ_ROLE:
> case LSM_SUBJ_TYPE:
> - rc = security_filter_rule_match(secid,
> + lsmblob_init(&blob, secid);
> + rc = security_filter_rule_match(&blob,
On the bprm hook, IMA calls process_measurement() twice. The first
time the secid is passed as an argument based on a call to
security_task_getsecid(), while the second time it is based on
security_cred_getsecid(). process_measurement() passes the correct
secid converted to a blob, but instead of using the passed variable,
this code uses the locally defined blob field. A later patch removes
the the lsmblob_init(), leaving the local blob uninitialized.
Something is terribly wrong here.
Mimi
> rule->lsm[i].type,
> Audit_equal,
> rule->lsm[i].rule);
>
