Re: [PATCH 1/2] security: safesetid: use real GID for GID policy lookup

["Serge E. Hallyn" <serge@hallyn.com>]

On Sat, Oct 03, 2026 at 07:34:15PM +0900, tjdqudcks0424@naver.com wrote:
> From: Sung Byeongchan <tjdqudcks0424@naver.com>
> 
> SafeSetID policies constrain the ID transitions available to a task for
> each source ID.  id_permitted_for_cred() always builds that source key
> from the old real UID, even when it is checking a GID transition.  If the
> real UID and real GID differ, the policy attached to the real GID is
> therefore missed and the lookup can return the unconstrained default.
> 
> On current mainline, a test task with real UID 1000, real GID 2000 and
> only CAP_SETGID obtained non-allowlisted GID 2002 despite a 2000:2001 GID
> policy.  The bypass reproduced for setgid(), setegid(), setregid(),
> setresgid(), setfsgid() and setgroups() across three clean QEMU boots,
> and enabled access to a synthetic group-protected resource.  This shows
> a SafeSetID GID policy bypass and group privilege expansion; it does not
> show direct UID 0 elevation or a universal local privilege escalation.
> 
> Build the source key from the old real UID for UID policy checks and the
> old real GID for GID policy checks.  In three fixed-kernel boots, all
> tested non-allowlisted transitions were blocked while the allowed target,
> existing-ID, no-policy and UID-policy controls, and the existing SafeSetID
> selftest continued to pass.
> 
> Fixes: 5294bac97e12 ("LSM: SafeSetID: Add GID security policy handling")
> Cc: stable@vger.kernel.org
> Assisted-by: OpenAI Codex
> Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>

Seems obviously correct.  Thanks for the find and the fix.

Reviewed-by: Serge Hallyn <serge@hallyn.com>

> ---
>  security/safesetid/lsm.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index d5fb949050dd8..18124fc499dbc 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -148,13 +148,16 @@ static int safesetid_security_capable(const struct cred *cred,
>  static bool id_permitted_for_cred(const struct cred *old, kid_t new_id, enum setid_type new_type)
>  {
>  	bool permitted;
> +	kid_t source_id;
>  
>  	/* If our old creds already had this ID in it, it's fine. */
>  	if (new_type == UID) {
> +		source_id.uid = old->uid;
>  		if (uid_eq(new_id.uid, old->uid) || uid_eq(new_id.uid, old->euid) ||
>  			uid_eq(new_id.uid, old->suid))
>  			return true;
>  	} else if (new_type == GID){
> +		source_id.gid = old->gid;
>  		if (gid_eq(new_id.gid, old->gid) || gid_eq(new_id.gid, old->egid) ||
>  			gid_eq(new_id.gid, old->sgid))
>  			return true;
> @@ -162,11 +165,10 @@ static bool id_permitted_for_cred(const struct cred *old, kid_t new_id, enum set
>  		return false;
>  
>  	/*
> -	 * Transitions to new UIDs require a check against the policy of the old
> -	 * RUID.
> +	 * Transitions require a check against the policy of the old real ID.
>  	 */
>  	permitted =
> -	    setid_policy_lookup((kid_t){.uid = old->uid}, new_id, new_type) != SIDPOL_CONSTRAINED;
> +	    setid_policy_lookup(source_id, new_id, new_type) != SIDPOL_CONSTRAINED;
>  
>  	if (!permitted) {
>  		if (new_type == UID) {
> -- 
> 2.43.0