On 2018-05-17 17:31, Ondrej Mosnacek wrote:
> 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 incorrect also because it compared
> to cred->fsgid and not cred->gid.
> 
> GitHub issue:
> https://github.com/linux-audit/audit-kernel/issues/82
> 
> Fixes: 37eebe39c973 ("audit: improve GID/EGID comparation logic")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ondrej Mosnacek <omosn...@redhat.com>

Nice.  You found a function that let you not have to roll a new global
one.  Is the comparision with cred->fsgid important?

If you run ./scripts/checkpatch.pl on the patch file it will complain on
those lines longer than 80 chars.  I don't have a problem with it.


> ---
>  kernel/auditsc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index cbab0da86d15..ec38e4d97c23 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -490,20 +490,20 @@ static int audit_filter_rules(struct task_struct *tsk,
>                       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:
> -- 
> 2.17.0
> 

- RGB

--
Richard Guy Briggs <r...@redhat.com>
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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to