On 2018-05-31 09:19, Ondrej Mosnacek wrote: > The audit_filter_rules() function in auditsc.c used the in_[e]group_p() > functions to check GID/EGID match, but these functions use the current > task's credentials, while the comparison should use the credentials of > the task given to audit_filter_rules() as a parameter (tsk). > > Note that we can use group_search(cred->group_info, ...) as a > replacement for both in_group_p and in_egroup_p as these functions only > compare the parameter to cred->fsgid/egid and then call group_search. > > In fact, the usage of in_group_p was even more incorrect: it compares to > cred->fsgid (which is usually equal to cred->egid) and not cred->gid. > > GitHub issue: > https://github.com/linux-audit/audit-kernel/issues/82 > > Fixes: 37eebe39c973 ("audit: improve GID/EGID comparation logic") > Signed-off-by: Ondrej Mosnacek <[email protected]>
Given what you've turned up while investigating this, I'm comfortable with the solution you've presented. Reviewed-by: Richard Guy Briggs <[email protected]> > --- > kernel/auditsc.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index ceb1c4596c51..3a324ca2fd20 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -492,23 +492,21 @@ static int audit_filter_rules(struct task_struct *tsk, > break; > case AUDIT_GID: > result = audit_gid_comparator(cred->gid, f->op, f->gid); > - if (f->op == Audit_equal) { > - if (!result) > - result = in_group_p(f->gid); > - } else if (f->op == Audit_not_equal) { > - if (result) > - result = !in_group_p(f->gid); > - } > + if (f->op == Audit_equal) > + result = result || > + groups_search(cred->group_info, f->gid); > + else if (f->op == Audit_not_equal) > + result = result && > + !groups_search(cred->group_info, > f->gid); > break; > case AUDIT_EGID: > result = audit_gid_comparator(cred->egid, f->op, > f->gid); > - if (f->op == Audit_equal) { > - if (!result) > - result = in_egroup_p(f->gid); > - } else if (f->op == Audit_not_equal) { > - if (result) > - result = !in_egroup_p(f->gid); > - } > + if (f->op == Audit_equal) > + result = result || > + groups_search(cred->group_info, f->gid); > + else if (f->op == Audit_not_equal) > + result = result && > + !groups_search(cred->group_info, > f->gid); > break; > case AUDIT_SGID: > result = audit_gid_comparator(cred->sgid, f->op, > f->gid); > -- > 2.17.0 > - RGB -- Richard Guy Briggs <[email protected]> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
