Re: Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree

* Re: Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree
       [not found] <15382227856210@kroah.com>
@ 2018-10-01  8:29 ` Ondrej Mosnacek
  2018-10-02 11:50   ` Greg Kroah-Hartman
  2018-10-02 12:04   ` Paul Moore
  0 siblings, 2 replies; 3+ messages in thread
From: Ondrej Mosnacek @ 2018-10-01  8:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: alexander.levin, Paul Moore, stable-commits, stable

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.

>
>
> 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.

^ permalink raw reply	[flat|nested] 3+ messages in thread