Date: Sun, 03 Nov 2024 19:16:27 +0100 From: Olivier Certner <olce@freebsd.org> To: Cy Schubert <Cy.Schubert@cschubert.com> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 5169d4307eb9 - main - nfs: Fallback to GID_NOGROUP on no groups Message-ID: <2094473.mIT35NG9lc@ravel> In-Reply-To: <20241103171056.E8299280@slippy.cwsent.com> References: <202411031547.4A3Fl0Lh079122@gitrepo.freebsd.org> <20241103171056.E8299280@slippy.cwsent.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart1863566.1AojA6SiXe Content-Type: multipart/mixed; boundary="nextPart17542189.7ycxSsYEdt"; protected-headers="v1" Content-Transfer-Encoding: 7Bit From: Olivier Certner <olce@freebsd.org> To: Cy Schubert <Cy.Schubert@cschubert.com> Date: Sun, 03 Nov 2024 19:16:27 +0100 Message-ID: <2094473.mIT35NG9lc@ravel> In-Reply-To: <20241103171056.E8299280@slippy.cwsent.com> MIME-Version: 1.0 This is a multi-part message in MIME format. --nextPart17542189.7ycxSsYEdt Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="UTF-8" > I'm getting a different panic this time. Sigh... I see why, and it seems straightforward to fix. Could you please try the attached patch? It is also available at: https://people.freebsd.org/~olce/0001-cred-crsetgroups-Throw-away-old-groups-before-crexte.patch . Hopefully, this was the last problem to solve. All patches in the series, except the "nfs, rpc: *" one, have been tested and actually used on my machines for weeks. I however don't use NFS on a daily basis, and it is now pretty clear that I have a big problem in my NFS testing setup, or messed up the related tests (am still not sure). If you can't test this patch or if you still have a problem after this one, then I'll just revert the series pending my re-testing in a correctly setup NFS environment. Sorry to have dragged you in this debugging session, and thanks for your efforts and feedback. -- Olivier Certner --nextPart17542189.7ycxSsYEdt Content-Disposition: attachment; filename="0001-cred-crsetgroups-Throw-away-old-groups-before-crexte.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="x-UTF_8J"; name="0001-cred-crsetgroups-Throw-away-old-groups-before-crexte.patch" >From 02bd66dd2b7b9143b0801d232c52aa23df23136b Mon Sep 17 00:00:00 2001 From: Olivier Certner <olce@FreeBSD.org> Date: Sun, 3 Nov 2024 19:04:02 +0100 Subject: [PATCH] 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 will set the groups to an entirely new set anyway. This allows to reuse not shared credentials by resetting the group sets on them, which is necessary for certain kind of NFS exports. Fixes: ea26c0e79752 ("cred: crextend(): Harden, simplify") --- 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 b522edbf4e69..fb5e973b812f 100644 --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -2215,20 +2215,18 @@ crfree_final(struct ucred *cr) */ 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 the ABI. + */ 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; dest->cr_flags = src->cr_flags; crsetgroups(dest, src->cr_ngroups, src->cr_groups); uihold(dest->cr_uidinfo); uihold(dest->cr_ruidinfo); prison_hold(dest->cr_prison); @@ -2473,10 +2471,17 @@ void crsetgroups(struct ucred *cr, int ngrp, const gid_t *groups) { if (ngrp > ngroups_max + 1) ngrp = ngroups_max + 1; + /* + * crextend() expects 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 set them to a completely new list below, signal that we + * have thrown away the old ones. + */ + cr->cr_ngroups = 0; crextend(cr, ngrp); crsetgroups_internal(cr, ngrp, groups); groups_normalize(&cr->cr_ngroups, cr->cr_groups); } --nextPart17542189.7ycxSsYEdt-- --nextPart1863566.1AojA6SiXe Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQIzBAABCQAdFiEEmNCxHjkosai0LYIujKEwQJceJicFAmcnvfsACgkQjKEwQJce JicDTxAAgF7lvjTNdtDLxyWuwzWJd1L6Xii6eT+iS7+fAkxJGwdkTZY8rPhFLRim 1NNXJ0K4ksfDAAB7CSFmFIwFt4UngLckhdGOGDAtvKmx1skF+vd/Fqe70IaUN2jj Cz788wjhISPmJoY9dxHdfEwVzrxZwe9TZJmoqDkxV3C2fLs0B7ssOFpx8AJTh9EL 5EkNORo91XjZrH64MxvKcsrxzQrBqOYRwPG3pdQEJ8R5WF1Rw1tfkov9a8ndmPfA Epxi4bHetinb2EWVmKLl0S+lh0mmMk1WXWs/Cvf8AlYJzYc8LWDoq3a0G0SDGHQ2 KGq6diWtciu4NukwzOO8SIYZ6zqoaoLtDrrzQxOdX4IO83OBel0JSpiXFJGzNJw9 lKxMEHbBatWamwUqzSdU/+OUSQr8n2OgrT2Z3aJ/gsqEn4pNrli9O59DNP3j7sy2 ovMTTMgjIw/uAMQDi28Aw9nWXxIFyt8c6VACc2V/m0O7L/cz+Q6S2qBEcVn9vwVr bwEG1x6TZSNSIuwAUprlf2Dl6jKwgXbvXfRq4bLJoG8dU1NRPvYJDxG4TbJa/5N0 /y98u4R41vKzMYWcDQMdB0Pzu1VMNsOQDKBMVXhmBwjjpYfGER2TZ0tVYNy7OGYV PkEtSNvvMv+LSwqRwX7J35nd8xZ9071OF/YoUJ+9rkvqepbvWX0= =fN4W -----END PGP SIGNATURE----- --nextPart1863566.1AojA6SiXe--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2094473.mIT35NG9lc>