[PATCH bpf-next v4 10/20] lsm: Refactor return value of LSM hook audit_rule_match

Xu Kuohai
2024-07-11T11:14:08Z

From: Xu Kuohai <xukuohai@huawei.com>

To be consistent with most LSM hooks, convert the return value of
hook audit_rule_match to 0 or a negative error code.

Before:
- Hook audit_rule_match returns 1 if the rule matches, 0 if it not,
and negative error code otherwise.

After:
- Hook audit_rule_match returns 0 on success or a negative error
code on failure. An output parameter @match is introduced to hold
the match result on success.

Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
---
include/linux/lsm_hook_defs.h | 3 +-
security/apparmor/audit.c | 22 ++++++-------
security/apparmor/include/audit.h | 2 +-
security/security.c | 15 ++++++++-
security/selinux/include/audit.h | 8 +++--
security/selinux/ss/services.c | 54 +++++++++++++++++--------------
security/smack/smack_lsm.c | 19 +++++++----
7 files changed, 75 insertions(+), 48 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 54fec360947c..6b521744a23b 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -418,7 +418,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, u32 secid, u32 field, u32 op, void *lsmrule,
+ bool *match)
LSM_HOOK(void, LSM_RET_VOID, audit_rule_free, void *lsmrule)
#endif /* CONFIG_AUDIT */

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 6b5181c668b5..352a183b3325 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -264,11 +264,11 @@ 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(u32 sid, u32 field, u32 op, void *vrule, bool *match)
{
struct aa_audit_rule *rule = vrule;
struct aa_label *label;
- int found = 0;
+ bool found = false;

label = aa_secid_to_label(sid);

@@ -276,16 +276,14 @@ int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
return -ENOENT;

if (aa_label_is_subset(label, rule->label))
- found = 1;
+ found = true;
+
+ if (field == AUDIT_SUBJ_ROLE && op == Audit_equal)
+ *match = found;
+ else if (field == AUDIT_SUBJ_ROLE && op == Audit_not_equal)
+ *match = !found;
+ else
+ *match = false;

- switch (field) {
- case AUDIT_SUBJ_ROLE:
- switch (op) {
- case Audit_equal:
- return found;
- case Audit_not_equal:
- return !found;
- }
- }
return 0;
}
diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h
index 0c8cc86b417b..a227741f33c8 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(u32 sid, u32 field, u32 op, void *vrule, bool *match);

#endif /* __AA_AUDIT_H */
diff --git a/security/security.c b/security/security.c
index 2c161101074d..5e9de8d0cdde 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5450,7 +5450,20 @@ void security_audit_rule_free(void *lsmrule)
*/
int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule)
{
- return call_int_hook(audit_rule_match, secid, field, op, lsmrule);
+ int rc;
+ bool match = false;
+ struct security_hook_list *hp;
+
+ hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) {
+ rc = hp->hook.audit_rule_match(secid, field, op, lsmrule,
+ &match);
+ if (rc < 0)
+ return rc;
+ if (match)
+ break;
+ }
+
+ return match;
}
#endif /* CONFIG_AUDIT */

diff --git a/security/selinux/include/audit.h b/security/selinux/include/audit.h
index 29c7d4c86f6d..2d0799270426 100644
--- a/security/selinux/include/audit.h
+++ b/security/selinux/include/audit.h
@@ -45,11 +45,13 @@ void selinux_audit_rule_free(void *rule);
* @field: the field this rule refers to
* @op: the operator the rule uses
* @rule: pointer to the audit rule to check against
+ * @match: if the context id matches the rule
*
- * Returns 1 if the context id matches the rule, 0 if it does not, and
- * -errno on failure.
+ * Returns 0 on success and -errno on failure. @match holds the match
+ * result.
*/
-int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *rule);
+int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *rule,
+ bool *match);

/**
* 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..2946d28a25b1 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3633,29 +3633,32 @@ 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(u32 sid, u32 field, u32 op, void *vrule,
+ bool *match)
{
struct selinux_state *state = &selinux_state;
struct selinux_policy *policy;
struct context *ctxt;
struct mls_level *level;
struct selinux_audit_rule *rule = vrule;
- int match = 0;
+ int rc = 0;

if (unlikely(!rule)) {
WARN_ONCE(1, "selinux_audit_rule_match: missing rule\n");
return -ENOENT;
}

- if (!selinux_initialized())
+ if (!selinux_initialized()) {
+ *match = false;
return 0;
+ }

rcu_read_lock();

policy = rcu_dereference(state->policy);

if (rule->au_seqno < policy->latest_granting) {
- match = -ESTALE;
+ rc = -ESTALE;
goto out;
}

@@ -3663,7 +3666,7 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
if (unlikely(!ctxt)) {
WARN_ONCE(1, "selinux_audit_rule_match: unrecognized SID %d\n",
sid);
- match = -ENOENT;
+ rc = -ENOENT;
goto out;
}

@@ -3674,10 +3677,10 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
case AUDIT_OBJ_USER:
switch (op) {
case Audit_equal:
- match = (ctxt->user == rule->au_ctxt.user);
+ rc = (ctxt->user == rule->au_ctxt.user);
break;
case Audit_not_equal:
- match = (ctxt->user != rule->au_ctxt.user);
+ rc = (ctxt->user != rule->au_ctxt.user);
break;
}
break;
@@ -3685,10 +3688,10 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
case AUDIT_OBJ_ROLE:
switch (op) {
case Audit_equal:
- match = (ctxt->role == rule->au_ctxt.role);
+ rc = (ctxt->role == rule->au_ctxt.role);
break;
case Audit_not_equal:
- match = (ctxt->role != rule->au_ctxt.role);
+ rc = (ctxt->role != rule->au_ctxt.role);
break;
}
break;
@@ -3696,10 +3699,10 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
case AUDIT_OBJ_TYPE:
switch (op) {
case Audit_equal:
- match = (ctxt->type == rule->au_ctxt.type);
+ rc = (ctxt->type == rule->au_ctxt.type);
break;
case Audit_not_equal:
- match = (ctxt->type != rule->au_ctxt.type);
+ rc = (ctxt->type != rule->au_ctxt.type);
break;
}
break;
@@ -3712,39 +3715,42 @@ int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule)
&ctxt->range.level[0] : &ctxt->range.level[1]);
switch (op) {
case Audit_equal:
- match = mls_level_eq(&rule->au_ctxt.range.level[0],
- level);
+ rc = mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
break;
case Audit_not_equal:
- match = !mls_level_eq(&rule->au_ctxt.range.level[0],
- level);
+ rc = !mls_level_eq(&rule->au_ctxt.range.level[0],
+ level);
break;
case Audit_lt:
- match = (mls_level_dom(&rule->au_ctxt.range.level[0],
- level) &&
+ rc = (mls_level_dom(&rule->au_ctxt.range.level[0],
+ level) &&
!mls_level_eq(&rule->au_ctxt.range.level[0],
level));
break;
case Audit_le:
- match = mls_level_dom(&rule->au_ctxt.range.level[0],
- level);
+ rc = mls_level_dom(&rule->au_ctxt.range.level[0],
+ level);
break;
case Audit_gt:
- match = (mls_level_dom(level,
- &rule->au_ctxt.range.level[0]) &&
+ rc = (mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]) &&
!mls_level_eq(level,
&rule->au_ctxt.range.level[0]));
break;
case Audit_ge:
- match = mls_level_dom(level,
- &rule->au_ctxt.range.level[0]);
+ rc = mls_level_dom(level,
+ &rule->au_ctxt.range.level[0]);
break;
}
}

out:
rcu_read_unlock();
- return match;
+ if (rc < 0)
+ return rc;
+ *match = !!rc;
+ return 0;
}

static int aurule_avc_callback(u32 event)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 9a121ad53b16..ea0f0cf11ff3 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4764,11 +4764,15 @@ static int smack_audit_rule_known(struct audit_krule *krule)
* @field: audit rule flags given from user-space
* @op: required testing operator
* @vrule: smack internal rule presentation
+ * @match: the match result
*
* The core Audit hook. It's used to take the decision of
* whether to audit or not to audit a given object.
+ *
+ * Returns 0 on success or negative error code on failure.
*/
-static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule)
+static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
+ bool *match)
{
struct smack_known *skp;
char *rule = vrule;
@@ -4778,8 +4782,10 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule)
return -ENOENT;
}

- if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER)
+ if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER) {
+ *match = false;
return 0;
+ }

skp = smack_from_secid(secid);

@@ -4789,10 +4795,11 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule)
* label.
*/
if (op == Audit_equal)
- return (rule == skp->smk_known);
- if (op == Audit_not_equal)
- return (rule != skp->smk_known);
-
+ *match = (rule == skp->smk_known);
+ else if (op == Audit_not_equal)
+ *match = (rule != skp->smk_known);
+ else
+ *match = false;
return 0;
}

--
2.30.2