* Re: Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree
2018-10-01 8:29 ` Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree Ondrej Mosnacek
@ 2018-10-02 11:50 ` Greg Kroah-Hartman
2018-10-02 12:04 ` Paul Moore
1 sibling, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-02 11:50 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: alexander.levin, Paul Moore, stable-commits, stable
On Mon, Oct 01, 2018 at 10:29:26AM +0200, Ondrej Mosnacek wrote:
> Hi,
>
> On Sat, Sep 29, 2018 at 2:10 PM <gregkh@linuxfoundation.org> wrote:
> > This is a note to let you know that I've just added the patch titled
> >
> > audit: Fix extended comparison of GID/EGID
> >
> > to the 4.18-stable tree which can be found at:
> > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> > audit-fix-extended-comparison-of-gid-egid.patch
> > and it can be found in the queue-4.18 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
>
> IIRC Paul didn't want this patch to go to stable (he asked me to
> remove the Cc: stable@... line), since the bug has been there for a
> long time and any user affected by it either doesn't care or might
> actually (maybe unknowingly) rely on it. I still kept the Fixes: line
> so it is clear which commit introduced the bug.
>
> Paul, any comments?
>
> In any case, if you decide to push this patch into stable (note that
> it is queued also for 4.14, 4.9, 4.4, and 3.18), then make sure to
> include also commit 4b09791ba059 ("cred: conditionally declare
> groups-related functions") to avoid build errors with
> CONFIG_MULTIUSER=n and CONFIG_AUDIT_SYSCALL=y. It is a non-functional
> commit for the rest of the kernel.
I've now dropped this from all of the stable queues, thanks.
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree
2018-10-01 8:29 ` Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree Ondrej Mosnacek
2018-10-02 11:50 ` Greg Kroah-Hartman
@ 2018-10-02 12:04 ` Paul Moore
1 sibling, 0 replies; 3+ messages in thread
From: Paul Moore @ 2018-10-02 12:04 UTC (permalink / raw)
To: omosnace; +Cc: gregkh, alexander.levin, stable-commits, stable
On Mon, Oct 1, 2018 at 4:29 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Hi,
>
> On Sat, Sep 29, 2018 at 2:10 PM <gregkh@linuxfoundation.org> wrote:
> > This is a note to let you know that I've just added the patch titled
> >
> > audit: Fix extended comparison of GID/EGID
> >
> > to the 4.18-stable tree which can be found at:
> > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> > audit-fix-extended-comparison-of-gid-egid.patch
> > and it can be found in the queue-4.18 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
>
> IIRC Paul didn't want this patch to go to stable (he asked me to
> remove the Cc: stable@... line), since the bug has been there for a
> long time and any user affected by it either doesn't care or might
> actually (maybe unknowingly) rely on it. I still kept the Fixes: line
> so it is clear which commit introduced the bug.
>
> Paul, any comments?
Greg does what Greg wants to do. I still agree with my earlier
comments that Ondrej is referencing, but if Greg wants to backport it
into his stable tree that's his decision to make.
> In any case, if you decide to push this patch into stable (note that
> it is queued also for 4.14, 4.9, 4.4, and 3.18), then make sure to
> include also commit 4b09791ba059 ("cred: conditionally declare
> groups-related functions") to avoid build errors with
> CONFIG_MULTIUSER=n and CONFIG_AUDIT_SYSCALL=y. It is a non-functional
> commit for the rest of the kernel.
>
> >
> >
> > From foo@baz Sat Sep 29 04:24:28 PDT 2018
> > From: "Ondrej Mosnáček" <omosnace@redhat.com>
> > Date: Tue, 5 Jun 2018 11:00:10 +0200
> > Subject: audit: Fix extended comparison of GID/EGID
> >
> > From: "Ondrej Mosnáček" <omosnace@redhat.com>
> >
> > [ Upstream commit af85d1772e31fed34165a1b3decef340cf4080c0 ]
> >
> > 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 <omosnace@redhat.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > kernel/auditsc.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -494,20 +494,20 @@ static int audit_filter_rules(struct tas
> > result = audit_gid_comparator(cred->gid, f->op, f->gid);
> > if (f->op == Audit_equal) {
> > if (!result)
> > - result = in_group_p(f->gid);
> > + result = groups_search(cred->group_info, f->gid);
> > } else if (f->op == Audit_not_equal) {
> > if (result)
> > - result = !in_group_p(f->gid);
> > + 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);
> > + result = groups_search(cred->group_info, f->gid);
> > } else if (f->op == Audit_not_equal) {
> > if (result)
> > - result = !in_egroup_p(f->gid);
> > + result = !groups_search(cred->group_info, f->gid);
> > }
> > break;
> > case AUDIT_SGID:
> >
> >
> > Patches currently in stable-queue which might be from omosnace@redhat.com are
> >
> > queue-4.18/audit-fix-extended-comparison-of-gid-egid.patch
>
> Thanks,
>
> --
> Ondrej Mosnacek <omosnace at redhat dot com>
> Associate Software Engineer, Security Technologies
> Red Hat, Inc.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 3+ messages in thread