| Message ID | 20210715091724.45768-1-tianjia.zhang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Smack: Fix wrong semantics in smk_access_entry() | expand |
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@linux.alibaba.com> > --- > 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; > } > > /**
On 7/15/2021 8:15 AM, Casey Schaufler wrote: > 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@linux.alibaba.com> Added to the Smack next branch. >> --- >> 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; >> } >> >> /**
On 7/21/21 12:32 AM, Casey Schaufler wrote: > On 7/15/2021 8:15 AM, Casey Schaufler wrote: >> 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@linux.alibaba.com> > > Added to the Smack next branch. > Great, Thanks. Tianjia
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; } /**
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. Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> --- security/smack/smack_access.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)