Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Nov 2024 13:00:58 GMT
From:      Olivier Certner <olce@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: dcf34d8a828a - stable/13 - cred: crsetgroups(): Throw away old groups before crextend()
Message-ID:  <202411151300.4AFD0woV071542@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by olce:

URL: https://cgit.FreeBSD.org/src/commit/?id=dcf34d8a828aa2a369a2b738192180b8402ca319

commit dcf34d8a828aa2a369a2b738192180b8402ca319
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-11-03 18:04:02 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-11-15 12:59:10 +0000

    cred: crsetgroups(): Throw away old groups before crextend()
    
    Now that crextend() asserts that groups are not set (rightfully so,
    since it may change the backing storage without copying the content of
    the old one), have crsetgroups() throw away the old groups before
    calling it, as it installs an entirely new set anyway.
    
    This allows to reuse unshared credentials by resetting their groups set,
    as NFS exports actually do.
    
    Reported by:    cy
    Tested by:      cy
    Fixes:          ea26c0e79752 ("cred: crextend(): Harden, simplify")
    Pointy hat to:  olce
    
    While here, as I forgot these credits in commit 5169d4307eb9 ("nfs: Fallback to
    GID_NOGROUP on no groups"):
    Tested by:    cy, David Wolfskill (panics caused by mountd(8))
    Tested by:    kib (MINIMAL/custom kernel compile breakup)
    
    (cherry picked from commit 169a10853a50f9bbb037492e6f2737cce10f6b99)
    
    Approved by:    markj (mentor)
---
 sys/kern/kern_prot.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index e62efb285698..482472025fdd 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -2215,16 +2215,14 @@ void
 crcopy(struct ucred *dest, struct ucred *src)
 {
 
+	/*
+	 * Ideally, 'cr_ngroups' should be moved out of 'struct ucred''s bcopied
+	 * area, but this would break the ABI, so is deferred until there is
+	 * a compelling need to change it.
+	 */
 	bcopy(&src->cr_startcopy, &dest->cr_startcopy,
 	    (unsigned)((caddr_t)&src->cr_endcopy -
 		(caddr_t)&src->cr_startcopy));
-	/*
-	 * Avoids an assertion in crsetgroups() -> crextend().  Ideally,
-	 * 'cr_ngroups' should be moved out of 'struct ucred''s bcopied area,
-	 * but this would break the ABI, so is deferred until there is a real
-	 * need to change the ABI.
-	 */
-	dest->cr_ngroups = 0;
 	crsetgroups(dest, src->cr_ngroups, src->cr_groups);
 	uihold(dest->cr_uidinfo);
 	uihold(dest->cr_ruidinfo);
@@ -2483,6 +2481,13 @@ crsetgroups(struct ucred *cr, int ngrp, const gid_t *groups)
 
 	if (ngrp > ngroups_max + 1)
 		ngrp = ngroups_max + 1;
+	/*
+	 * crextend() asserts that groups are not set, as it may allocate a new
+	 * backing storage without copying the content of the old one.  Since we
+	 * are going to install a completely new set anyway, signal that we
+	 * consider the old ones thrown away.
+	 */
+	cr->cr_ngroups = 0;
 	crextend(cr, ngrp);
 	crsetgroups_internal(cr, ngrp, groups);
 	groups_normalize(&cr->cr_ngroups, cr->cr_groups);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202411151300.4AFD0woV071542>