Re: [PATCH] Smack: Fix wrong semantics in smk_access_entry()

From: Casey Schaufler
Date: Thu Jul 15 2021 - 11:15:24 EST


On 7/15/2021 2:17 AM, Tianjia Zhang wrote:
> In the smk_access_entry() function, if no matching rule is found
> in the rust_list, a negative error code will be used to perform bit
> operations with the MAY_ enumeration value. This is semantically
> wrong. This patch fixes this issue.

Indeed, the code as written is functioning correctly by
sheer luck. I will take this patch. Thank you.

>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@xxxxxxxxxxxxxxxxx>
> ---
> security/smack/smack_access.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
> index 1f391f6a3d47..d2186e2757be 100644
> --- a/security/smack/smack_access.c
> +++ b/security/smack/smack_access.c
> @@ -81,23 +81,22 @@ int log_policy = SMACK_AUDIT_DENIED;
> int smk_access_entry(char *subject_label, char *object_label,
> struct list_head *rule_list)
> {
> - int may = -ENOENT;
> struct smack_rule *srp;
>
> list_for_each_entry_rcu(srp, rule_list, list) {
> if (srp->smk_object->smk_known == object_label &&
> srp->smk_subject->smk_known == subject_label) {
> - may = srp->smk_access;
> - break;
> + int may = srp->smk_access;
> + /*
> + * MAY_WRITE implies MAY_LOCK.
> + */
> + if ((may & MAY_WRITE) == MAY_WRITE)
> + may |= MAY_LOCK;
> + return may;
> }
> }
>
> - /*
> - * MAY_WRITE implies MAY_LOCK.
> - */
> - if ((may & MAY_WRITE) == MAY_WRITE)
> - may |= MAY_LOCK;
> - return may;
> + return -ENOENT;
> }
>
> /**