| Message ID | 20240830003411.16818-3-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | LSM: Move away from secids | expand |
Hi Casey, kernel test robot noticed the following build warnings: [auto build test WARNING on pcmoore-audit/next] [also build test WARNING on pcmoore-selinux/next zohar-integrity/next-integrity linus/master v6.11-rc5 next-20240830] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Add-the-lsmblob-data-structure/20240830-085050 base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next patch link: https://lore.kernel.org/r/20240830003411.16818-3-casey%40schaufler-ca.com patch subject: [PATCH v2 02/13] LSM: Use lsmblob in security_audit_rule_match config: i386-randconfig-061-20240830 (https://download.01.org/0day-ci/archive/20240831/202408310649.X413mMQP-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240831/202408310649.X413mMQP-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408310649.X413mMQP-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> security/integrity/ima/ima_policy.c:654:53: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int [usertype] secid @@ got struct lsmblob * @@ security/integrity/ima/ima_policy.c:654:53: sparse: expected unsigned int [usertype] secid security/integrity/ima/ima_policy.c:654:53: sparse: got struct lsmblob * security/integrity/ima/ima_policy.c:663:53: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int [usertype] secid @@ got struct lsmblob * @@ security/integrity/ima/ima_policy.c:663:53: sparse: expected unsigned int [usertype] secid security/integrity/ima/ima_policy.c:663:53: sparse: got struct lsmblob * security/integrity/ima/ima_policy.c: note: in included file: include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true security/integrity/ima/ima_policy.c:1666:52: sparse: sparse: self-comparison always evaluates to false security/integrity/ima/ima_policy.c:1701:55: sparse: sparse: self-comparison always evaluates to false security/integrity/ima/ima_policy.c:1728:55: sparse: sparse: self-comparison always evaluates to false security/integrity/ima/ima_policy.c:1754:55: sparse: sparse: self-comparison always evaluates to false include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true vim +654 security/integrity/ima/ima_policy.c 553 554 /** 555 * ima_match_rules - determine whether an inode matches the policy rule. 556 * @rule: a pointer to a rule 557 * @idmap: idmap of the mount the inode was found from 558 * @inode: a pointer to an inode 559 * @cred: a pointer to a credentials structure for user validation 560 * @secid: the secid of the task to be validated 561 * @func: LIM hook identifier 562 * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC) 563 * @func_data: func specific data, may be NULL 564 * 565 * Returns true on rule match, false on failure. 566 */ 567 static bool ima_match_rules(struct ima_rule_entry *rule, 568 struct mnt_idmap *idmap, 569 struct inode *inode, const struct cred *cred, 570 u32 secid, enum ima_hooks func, int mask, 571 const char *func_data) 572 { 573 int i; 574 bool result = false; 575 struct ima_rule_entry *lsm_rule = rule; 576 bool rule_reinitialized = false; 577 578 if ((rule->flags & IMA_FUNC) && 579 (rule->func != func && func != POST_SETATTR)) 580 return false; 581 582 switch (func) { 583 case KEY_CHECK: 584 case CRITICAL_DATA: 585 return ((rule->func == func) && 586 ima_match_rule_data(rule, func_data, cred)); 587 default: 588 break; 589 } 590 591 if ((rule->flags & IMA_MASK) && 592 (rule->mask != mask && func != POST_SETATTR)) 593 return false; 594 if ((rule->flags & IMA_INMASK) && 595 (!(rule->mask & mask) && func != POST_SETATTR)) 596 return false; 597 if ((rule->flags & IMA_FSMAGIC) 598 && rule->fsmagic != inode->i_sb->s_magic) 599 return false; 600 if ((rule->flags & IMA_FSNAME) 601 && strcmp(rule->fsname, inode->i_sb->s_type->name)) 602 return false; 603 if ((rule->flags & IMA_FSUUID) && 604 !uuid_equal(&rule->fsuuid, &inode->i_sb->s_uuid)) 605 return false; 606 if ((rule->flags & IMA_UID) && !rule->uid_op(cred->uid, rule->uid)) 607 return false; 608 if (rule->flags & IMA_EUID) { 609 if (has_capability_noaudit(current, CAP_SETUID)) { 610 if (!rule->uid_op(cred->euid, rule->uid) 611 && !rule->uid_op(cred->suid, rule->uid) 612 && !rule->uid_op(cred->uid, rule->uid)) 613 return false; 614 } else if (!rule->uid_op(cred->euid, rule->uid)) 615 return false; 616 } 617 if ((rule->flags & IMA_GID) && !rule->gid_op(cred->gid, rule->gid)) 618 return false; 619 if (rule->flags & IMA_EGID) { 620 if (has_capability_noaudit(current, CAP_SETGID)) { 621 if (!rule->gid_op(cred->egid, rule->gid) 622 && !rule->gid_op(cred->sgid, rule->gid) 623 && !rule->gid_op(cred->gid, rule->gid)) 624 return false; 625 } else if (!rule->gid_op(cred->egid, rule->gid)) 626 return false; 627 } 628 if ((rule->flags & IMA_FOWNER) && 629 !rule->fowner_op(i_uid_into_vfsuid(idmap, inode), 630 rule->fowner)) 631 return false; 632 if ((rule->flags & IMA_FGROUP) && 633 !rule->fgroup_op(i_gid_into_vfsgid(idmap, inode), 634 rule->fgroup)) 635 return false; 636 for (i = 0; i < MAX_LSM_RULES; i++) { 637 int rc = 0; 638 struct lsmblob blob = { }; 639 640 if (!lsm_rule->lsm[i].rule) { 641 if (!lsm_rule->lsm[i].args_p) 642 continue; 643 else 644 return false; 645 } 646 647 retry: 648 switch (i) { 649 case LSM_OBJ_USER: 650 case LSM_OBJ_ROLE: 651 case LSM_OBJ_TYPE: 652 /* scaffolding */ 653 security_inode_getsecid(inode, &blob.scaffold.secid); > 654 rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type, 655 Audit_equal, 656 lsm_rule->lsm[i].rule); 657 break; 658 case LSM_SUBJ_USER: 659 case LSM_SUBJ_ROLE: 660 case LSM_SUBJ_TYPE: 661 /* scaffolding */ 662 blob.scaffold.secid = secid; 663 rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type, 664 Audit_equal, 665 lsm_rule->lsm[i].rule); 666 break; 667 default: 668 break; 669 } 670 671 if (rc == -ESTALE && !rule_reinitialized) { 672 lsm_rule = ima_lsm_copy_rule(rule, GFP_ATOMIC); 673 if (lsm_rule) { 674 rule_reinitialized = true; 675 goto retry; 676 } 677 } 678 if (!rc) { 679 result = false; 680 goto out; 681 } 682 } 683 result = true; 684 685 out: 686 if (rule_reinitialized) { 687 for (i = 0; i < MAX_LSM_RULES; i++) 688 ima_filter_rule_free(lsm_rule->lsm[i].rule); 689 kfree(lsm_rule); 690 } 691 return result; 692 } 693
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 855db460e08b..1d3bdf71109e 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -416,7 +416,8 @@ LSM_HOOK(void, LSM_RET_VOID, key_post_create_or_update, struct key *keyring, LSM_HOOK(int, 0, audit_rule_init, u32 field, u32 op, char *rulestr, void **lsmrule, gfp_t gfp) LSM_HOOK(int, 0, audit_rule_known, struct audit_krule *krule) -LSM_HOOK(int, 0, audit_rule_match, u32 secid, u32 field, u32 op, void *lsmrule) +LSM_HOOK(int, 0, audit_rule_match, struct lsmblob *blob, u32 field, u32 op, + void *lsmrule) LSM_HOOK(void, LSM_RET_VOID, audit_rule_free, void *lsmrule) #endif /* CONFIG_AUDIT */ diff --git a/include/linux/security.h b/include/linux/security.h index 0057a22137e8..c0ed2119a622 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2071,7 +2071,8 @@ static inline void security_key_post_create_or_update(struct key *keyring, int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule, gfp_t gfp); int security_audit_rule_known(struct audit_krule *krule); -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *lsmrule); void security_audit_rule_free(void *lsmrule); #else @@ -2087,8 +2088,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) return 0; } -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, - void *lsmrule) +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, + u32 op, void *lsmrule) { return 0; } diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index d6ef4f4f9cba..c4c7cda3b846 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -1339,8 +1339,8 @@ int audit_filter(int msgtype, unsigned int listtype) for (i = 0; i < e->rule.field_count; i++) { struct audit_field *f = &e->rule.fields[i]; + struct lsmblob blob = { }; pid_t pid; - u32 sid; switch (f->type) { case AUDIT_PID: @@ -1370,9 +1370,12 @@ int audit_filter(int msgtype, unsigned int listtype) case AUDIT_SUBJ_SEN: case AUDIT_SUBJ_CLR: if (f->lsm_rule) { - security_current_getsecid_subj(&sid); - result = security_audit_rule_match(sid, - f->type, f->op, f->lsm_rule); + /* scaffolding */ + security_current_getsecid_subj( + &blob.scaffold.secid); + result = security_audit_rule_match( + &blob, f->type, f->op, + f->lsm_rule); } break; case AUDIT_EXE: diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 6f0d6fb6523f..23adb15cae43 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -471,6 +471,7 @@ static int audit_filter_rules(struct task_struct *tsk, const struct cred *cred; int i, need_sid = 1; u32 sid; + struct lsmblob blob = { }; unsigned int sessionid; if (ctx && rule->prio <= ctx->prio) @@ -681,7 +682,10 @@ static int audit_filter_rules(struct task_struct *tsk, security_current_getsecid_subj(&sid); need_sid = 0; } - result = security_audit_rule_match(sid, f->type, + /* scaffolding */ + blob.scaffold.secid = sid; + result = security_audit_rule_match(&blob, + f->type, f->op, f->lsm_rule); } @@ -696,15 +700,19 @@ static int audit_filter_rules(struct task_struct *tsk, if (f->lsm_rule) { /* Find files that match */ if (name) { + /* scaffolding */ + blob.scaffold.secid = name->osid; result = security_audit_rule_match( - name->osid, + &blob, f->type, f->op, f->lsm_rule); } else if (ctx) { list_for_each_entry(n, &ctx->names_list, list) { + /* scaffolding */ + blob.scaffold.secid = n->osid; if (security_audit_rule_match( - n->osid, + &blob, f->type, f->op, f->lsm_rule)) { @@ -716,7 +724,9 @@ static int audit_filter_rules(struct task_struct *tsk, /* Find ipc objects that match */ if (!ctx || ctx->type != AUDIT_IPC) break; - if (security_audit_rule_match(ctx->ipc.osid, + /* scaffolding */ + blob.scaffold.secid = ctx->ipc.osid; + if (security_audit_rule_match(&blob, f->type, f->op, f->lsm_rule)) ++result; diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c index 6b5181c668b5..758b75a9c1c5 100644 --- a/security/apparmor/audit.c +++ b/security/apparmor/audit.c @@ -264,13 +264,17 @@ int aa_audit_rule_known(struct audit_krule *rule) return 0; } -int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) +int aa_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, void *vrule) { struct aa_audit_rule *rule = vrule; struct aa_label *label; int found = 0; - label = aa_secid_to_label(sid); + /* scaffolding */ + if (!blob->apparmor.label && blob->scaffold.secid) + label = aa_secid_to_label(blob->scaffold.secid); + else + label = blob->apparmor.label; if (!label) return -ENOENT; diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 0c8cc86b417b..c5a516e61318 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -202,6 +202,6 @@ static inline int complain_error(int error) void aa_audit_rule_free(void *vrule); int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule, gfp_t gfp); int aa_audit_rule_known(struct audit_krule *rule); -int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule); +int aa_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, void *vrule); #endif /* __AA_AUDIT_H */ diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 09da8e639239..40119816b848 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -635,7 +635,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, return false; for (i = 0; i < MAX_LSM_RULES; i++) { int rc = 0; - u32 osid; + struct lsmblob blob = { }; if (!lsm_rule->lsm[i].rule) { if (!lsm_rule->lsm[i].args_p) @@ -649,15 +649,18 @@ static bool ima_match_rules(struct ima_rule_entry *rule, case LSM_OBJ_USER: case LSM_OBJ_ROLE: case LSM_OBJ_TYPE: - security_inode_getsecid(inode, &osid); - rc = ima_filter_rule_match(osid, lsm_rule->lsm[i].type, + /* scaffolding */ + security_inode_getsecid(inode, &blob.scaffold.secid); + rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type, Audit_equal, lsm_rule->lsm[i].rule); break; case LSM_SUBJ_USER: case LSM_SUBJ_ROLE: case LSM_SUBJ_TYPE: - rc = ima_filter_rule_match(secid, lsm_rule->lsm[i].type, + /* scaffolding */ + blob.scaffold.secid = secid; + rc = ima_filter_rule_match(&blob, lsm_rule->lsm[i].type, Audit_equal, lsm_rule->lsm[i].rule); break; diff --git a/security/security.c b/security/security.c index 8cee5b6c6e6d..64a6d6bbd1f4 100644 --- a/security/security.c +++ b/security/security.c @@ -5399,7 +5399,7 @@ void security_audit_rule_free(void *lsmrule) /** * security_audit_rule_match() - Check if a label matches an audit rule - * @secid: security label + * @lsmblob: security label * @field: LSM audit field * @op: matching operator * @lsmrule: audit rule @@ -5410,9 +5410,10 @@ void security_audit_rule_free(void *lsmrule) * Return: Returns 1 if secid matches the rule, 0 if it does not, -ERRNO on * failure. */ -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *lsmrule) { - return call_int_hook(audit_rule_match, secid, field, op, lsmrule); + return call_int_hook(audit_rule_match, blob, field, op, lsmrule); } #endif /* CONFIG_AUDIT */ diff --git a/security/selinux/include/audit.h b/security/selinux/include/audit.h index 29c7d4c86f6d..104165e4c931 100644 --- a/security/selinux/include/audit.h +++ b/security/selinux/include/audit.h @@ -41,7 +41,7 @@ void selinux_audit_rule_free(void *rule); /** * selinux_audit_rule_match - determine if a context ID matches a rule. - * @sid: the context ID to check + * @blob: includes the context ID to check * @field: the field this rule refers to * @op: the operator the rule uses * @rule: pointer to the audit rule to check against @@ -49,7 +49,8 @@ void selinux_audit_rule_free(void *rule); * Returns 1 if the context id matches the rule, 0 if it does not, and * -errno on failure. */ -int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *rule); +int selinux_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *rule); /** * selinux_audit_rule_known - check to see if rule contains selinux fields. diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index e33e55384b75..43eb1d46942c 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -3633,7 +3633,8 @@ int selinux_audit_rule_known(struct audit_krule *rule) return 0; } -int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) +int selinux_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *vrule) { struct selinux_state *state = &selinux_state; struct selinux_policy *policy; @@ -3659,10 +3660,14 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule) goto out; } - ctxt = sidtab_search(policy->sidtab, sid); + /* scaffolding */ + if (!blob->selinux.secid && blob->scaffold.secid) + blob->selinux.secid = blob->scaffold.secid; + + ctxt = sidtab_search(policy->sidtab, blob->selinux.secid); if (unlikely(!ctxt)) { WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n", - sid); + blob->selinux.secid); match = -ENOENT; goto out; } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 4164699cd4f6..52d5ef986db8 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4776,7 +4776,7 @@ static int smack_audit_rule_known(struct audit_krule *krule) /** * smack_audit_rule_match - Audit given object ? - * @secid: security id for identifying the object to test + * @blob: security id for identifying the object to test * @field: audit rule flags given from user-space * @op: required testing operator * @vrule: smack internal rule presentation @@ -4784,7 +4784,8 @@ static int smack_audit_rule_known(struct audit_krule *krule) * The core Audit hook. It's used to take the decision of * whether to audit or not to audit a given object. */ -static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule) +static int smack_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *vrule) { struct smack_known *skp; char *rule = vrule; @@ -4797,7 +4798,11 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule) if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER) return 0; - skp = smack_from_secid(secid); + /* scaffolding */ + if (!blob->smack.skp && blob->scaffold.secid) + skp = smack_from_secid(blob->scaffold.secid); + else + skp = blob->smack.skp; /* * No need to do string comparisons. If a match occurs,
Change the secid parameter of security_audit_rule_match to a lsmblob structure pointer. Pass the entry from the lsmblob structure for the approprite slot to the LSM hook. Change the users of security_audit_rule_match to use the lsmblob instead of a u32. The scaffolding function lsmblob_init() fills the blob with the value of the old secid, ensuring that it is available to the appropriate module hook. The sources of the secid, security_task_getsecid() and security_inode_getsecid(), will be converted to use the blob structure later in the series. At the point the use of lsmblob_init() is dropped. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- include/linux/lsm_hook_defs.h | 3 ++- include/linux/security.h | 7 ++++--- kernel/auditfilter.c | 11 +++++++---- kernel/auditsc.c | 18 ++++++++++++++---- security/apparmor/audit.c | 8 ++++++-- security/apparmor/include/audit.h | 2 +- security/integrity/ima/ima_policy.c | 11 +++++++---- security/security.c | 7 ++++--- security/selinux/include/audit.h | 5 +++-- security/selinux/ss/services.c | 11 ++++++++--- security/smack/smack_lsm.c | 11 ++++++++--- 11 files changed, 64 insertions(+), 30 deletions(-)