Re: Patch "audit: Fix extended comparison of GID/EGID" has been added to the 4.18-stable tree
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/EGIDto the 4.18-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
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.
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/EGIDto the 4.18-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
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
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/EGIDto the 4.18-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
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.
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Ondrej Mosnacek -
Paul Moore