Hi, I noticed this suspicious line in the definition of the audit_filter_rules function in auditsc.c: [...] case AUDIT_SESSIONID: sessionid = audit_get_sessionid(current); // <--- HERE result = audit_comparator(sessionid, f->op, f->val); break; [...] Here, the sessionid is retrieved from the current task pointer, while all the other code in this function compares against the tsk task pointer. It seems that it is not always guaranteed that tsk == current, so my question is: Is it intentional for some reason or should it be tsk instead of current? Thanks, -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.

On 2018-05-16 10:43, Ondrej Mosnacek wrote: > I found more inconsistencies: > [...] > 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); > } > 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); > } > break; > [...] > > The in_[e]group_p functions match the current task's group list. > Unfortunately there don't seem to be functions in the kernel that > would do the same for arbitrary struct cred pointers, we may need to > add these to fix this. > > See the definition of in_group_p for reference: > https://elixir.bootlin.com/linux/latest/source/kernel/groups.c#L219 Interesting. Nice catch. I'll need to look at these and the previous one again. File github audit kernel issues...

> 2018-05-16 8:57 GMT+02:00 Ondrej Mosnacek <omosnace(a)redhat.com&gt;: > > Hi, > > > > I noticed this suspicious line in the definition of the > > audit_filter_rules function in auditsc.c: > > > > [...] > > case AUDIT_SESSIONID: > > sessionid = audit_get_sessionid(current); // <--- HERE > > result = audit_comparator(sessionid, f->op, f->val); > > break; > > [...] > > > > Here, the sessionid is retrieved from the current task pointer, while > > all the other code in this function compares against the tsk task > > pointer. It seems that it is not always guaranteed that tsk == > > current, so my question is: Is it intentional for some reason or > > should it be tsk instead of current? > > > > Ondrej Mosnacek <omosnace at redhat dot com> > > Ondrej Mosnacek <omosnace at redhat dot com> - RGB -- Richard Guy Briggs <rgb(a)redhat.com&gt; 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