2018-05-18 21:23 GMT+02:00 Paul Moore <paul@xxxxxxxxxxxxxx>: > On Thu, May 17, 2018 at 11:31 AM, Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: >> The audit_filter_rules() function in auditsc.c compared the session ID >> with the credentials of the current task, while it should use the >> credentials of the task given to audit_filter_rules() as a parameter >> (tsk). >> >> GitHub issue: >> https://github.com/linux-audit/audit-kernel/issues/82 >> >> Fixes: 8fae47705685 ("audit: add support for session ID user filter") >> Cc: stable@xxxxxxxxxxxxxxx >> Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> >> --- >> kernel/auditsc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Good catch. However, I'm not completely convinced this is stable material. > > This bug really only affects "task" filters which I believe to be > fairly limited in the wild, due to the limited number of fields that > one can filter on. Every other filter, including the "exit" filter, > work as expected (which is why the audit-testsuite didn't catch this > bug). Further, even in the case of the task filter, the *only* time > where the current session ID would be different from the tsk session > ID is in the case of a login event, but even in this case the two > values should be equal during the "task" filtering as you can't change > the login-ID/session-ID until after you have successfully > fork()'d/clone()'d the new task. > > I'll hold off on merging this in case I'm missing some even more > subtle point that you've found. From what I can tell this is a good > fix, but not something an actual user would ever really encounter and > therefore not something that warrants inclusion in stable. Fair enough, I don't have any objections. > >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index ec38e4d97c23..6d577a34b16b 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk, >> result = audit_gid_comparator(cred->fsgid, f->op, f->gid); >> break; >> case AUDIT_SESSIONID: >> - sessionid = audit_get_sessionid(current); >> + sessionid = audit_get_sessionid(tsk); >> result = audit_comparator(sessionid, f->op, f->val); >> break; >> case AUDIT_PERS: >> -- >> 2.17.0 > > -- > paul moore > www.paul-moore.com -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.
