Re: [PATCH ghak82] audit: Fix extended comparison of GID/EGID

OK, so after doing a bit of research I think this is how it is: 1. (Real) GID, effective GID, saved set-group-ID, FSGID and the list of supplementary groups are each a separate concept and IMHO should be each compared with a distinct field. 2. When checking access to a file, the FSGID and supplementary groups are what matters. 3. There is a hierarchy GID -> EGID -> FSGID, in that changing one of them changes the ones to right, but keeps the ones to the left (see setgid(2) and setfsgid(2)). 4. The saved set-group-ID is Linux-specific and is only checked when a process tries to change its real GID (so that it can regain the effective group ID established at the last exec call [1]). 5. The FSGID is also Linux-specific and was only necessary in Linux pre-2.0 in applications such as NFS where changing EGID would lead to security issues. [2] In an ideal world we should have fields: GID, EGID, FSGID, SGID, and possibly something like SUPPLGID, which would be only used when filtering events in the kernel and would not be emitted in records (or it would contain the list of all supplementary groups of the process). But, since we are not in and ideal world, we are now in a situation where: 1. Due to commit 37eebe39c973, we filter also by supplementary groups in both GID and EGID. 2. Due to the confusing naming/implementation/intent of the in_group_p function we also check for FSGID under GID. This is IMHO completely pointless (if anything, EGID is more related to FSGID than GID). Anyway, it is very rare for a process to change its FSGID so this bug (or its fix) should have very little to no impact in real life. The supplementary groups checking OTOH, since this is a feature that may easily significantly affect filtering (and has been there for over 6 years already), should be preserved for backwards compatibility. All things said, I still believe the patch should be applied, but of course it's up to Paul to decide. Either way it probably shouldn't go to stable for the reasons Paul described in the other patch ([3]).