From nobody Tue Sep 10 01:31:55 2024 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4X2mQN1JpDz5WB21; Tue, 10 Sep 2024 01:31:56 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4X2mQN0sL2z4fSc; Tue, 10 Sep 2024 01:31:56 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1725931916; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=i6Z0Rrf2S3MBVrwFgSJxyKDDjAfBZw8uQylpm2MCJRE=; b=C/8SnGyyfQzCdP1w4Arq90mYtVVce8KmFP9p8XcPs23fS0jZGQUTNomt3qOvTLNLNO0VI4 rjxWj1rVHfbtIgbWuXRBwh8ixsnnjf2HVqhRk2GiWiupLw0IvrLBaKSwTbyi0PoNzQYPXV qAm1vFgPFNOYMTnojjGQAcNcxhpDQ9vHNc9bRcr2FQmz4XziklDjmPNNfmSnR8dSEgeYm9 2VEkqWF9cCRtXqU0iK83g4GP9Jk9kWG/wwt+ZmajQcDEm0U539K1PvTvOONCVLz4alUWtH u/A2CIsaWM5krNcwxC+F9QbSSL/uNr08XpDoh9vkcJy3RHg9Ra2E9J7rlx+rcg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1725931916; a=rsa-sha256; cv=none; b=g4OA8j9uTeMUb8RlAREwZUomVak1+X81q3Ue7IAe70TAxRWv+NOyJd87+UG9OINOByTKuk Lewd8PZpyT9feLQ1Tfji4YZ6iG1mAZAZJ59ijSWC7hFzX6a4m4KEJmLsHWiftYqW9r/zrJ 7O6YLMYZxnYf2uRU1YtP34jzcjl/dXUJNvuj89bUQgLfQ633GaXUZa1ubMjDraaAUsIbVh ybYiitJsN28z5OBVY7B8kCTyXf3xI6nllFPhUJptzNNt6uMwS5/yqLqaaEpvUK24feeePh 48w5ux2VJhjgVk1ifcTTWAeH/4OTlEA5ECKtnmsHIwpGkGS+Q9fKsMg9SRK7tw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1725931916; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=i6Z0Rrf2S3MBVrwFgSJxyKDDjAfBZw8uQylpm2MCJRE=; b=bxKcrpHmtMY7B70f9Il/sUBK6g43JB0pzmTH275shsaZUCdbN56sizcwZDOXpnFO/rnH8b Jk4NSW6nCvqv11I4hbs8zE4aCxPEOd2uUUecICbjJE8HZnB9or0kRbs9VSJFx3khXCBeCC 6LJDG0Vvt9zWPYjpyVcKbGTSlRjcKWe0UmFUjkGnV+ZkksBHBZzvQzXAMcg8oRddkytjj2 J3TlcjuQm+8KOdQaeQiGN5Kpwn2r8tcRW7U5XBB3M/ZFZ3L7HucjRBUGUOWUMuKX7gn9+/ EtC9R0K9qxWjODDTS1Tun4BphqmhrT5U2Qqng52p3ho8UVP8jJcVbiOpojr+QQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4X2mQN08zmzRV4; Tue, 10 Sep 2024 01:31:56 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 48A1Vtic009222; Tue, 10 Sep 2024 01:31:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 48A1Vt5M009204; Tue, 10 Sep 2024 01:31:55 GMT (envelope-from git) Date: Tue, 10 Sep 2024 01:31:55 GMT Message-Id: <202409100131.48A1Vt5M009204@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Rick Macklem Subject: git: 617a9b99a594 - stable/14 - nfsd: Fix handling of NFSv4 setable attributes List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 617a9b99a594532095c9fed0cf8e4f728e24f480 Auto-Submitted: auto-generated The branch stable/14 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=617a9b99a594532095c9fed0cf8e4f728e24f480 commit 617a9b99a594532095c9fed0cf8e4f728e24f480 Author: Rick Macklem AuthorDate: 2024-08-27 21:19:33 +0000 Commit: Rick Macklem CommitDate: 2024-09-10 01:30:23 +0000 nfsd: Fix handling of NFSv4 setable attributes Commit d8a5961 made a change to nfsv4_sattr() that broke parsing of the setable attributes for a NFSv4 SETATTR. (It broke out of the code by setting "error" and returning right away, instead of noting the error in nd_repstat and allowing parsing of the attributes to continue.) By returning prematurely, it was possible for SETATTR to return the error, but with a bogus set of attribute bits set, since "retbits" had not yet been set to all zeros. (I am not sure if any client could be affected by this bug. The patch was done for a failure case detected by a pynfs test suite and not an actual client.) While here, the patch also fixes a few cases where the value of attributes gets set for attributes after an error has been set in nd_repstat. This would not really break the protocol, since a SETATTR is allowed to set some attributes and still return an failure, but should not really be done. (cherry picked from commit 5037c6398b2327366494a0434a894dc17ba8d023) --- sys/fs/nfsserver/nfs_nfsdport.c | 58 ++++++++++++++++++++++++----------------- sys/fs/nfsserver/nfs_nfsdserv.c | 2 +- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c index 3769fdc4ff73..767bdcd80709 100644 --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -3027,6 +3027,8 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, /* * Loop around getting the setable attributes. If an unsupported * one is found, set nd_repstat == NFSERR_ATTRNOTSUPP and return. + * Once nd_repstat != 0, do not set the attribute value, but keep + * parsing the attribute(s). */ if (retnotsup) { nd->nd_repstat = NFSERR_ATTRNOTSUPP; @@ -3044,12 +3046,13 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, switch (bitpos) { case NFSATTRBIT_SIZE: NFSM_DISSECT(tl, u_int32_t *, NFSX_HYPER); - if (vp != NULL && vp->v_type != VREG) { - error = (vp->v_type == VDIR) ? NFSERR_ISDIR : - NFSERR_INVAL; - goto nfsmout; + if (!nd->nd_repstat) { + if (vp != NULL && vp->v_type != VREG) + nd->nd_repstat = (vp->v_type == VDIR) ? + NFSERR_ISDIR : NFSERR_INVAL; + else + nvap->na_size = fxdr_hyper(tl); } - nvap->na_size = fxdr_hyper(tl); attrsum += NFSX_HYPER; break; case NFSATTRBIT_ACL: @@ -3086,7 +3089,8 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, case NFSATTRBIT_MODE: moderet = NFSERR_INVAL; /* Can't do MODESETMASKED. */ NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); - nvap->na_mode = nfstov_mode(*tl); + if (!nd->nd_repstat) + nvap->na_mode = nfstov_mode(*tl); attrsum += NFSX_UNSIGNED; break; case NFSATTRBIT_OWNER: @@ -3154,10 +3158,11 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, attrsum += NFSX_UNSIGNED; if (fxdr_unsigned(int, *tl)==NFSV4SATTRTIME_TOCLIENT) { NFSM_DISSECT(tl, u_int32_t *, NFSX_V4TIME); - fxdr_nfsv4time(tl, &nvap->na_atime); + if (!nd->nd_repstat) + fxdr_nfsv4time(tl, &nvap->na_atime); toclient = 1; attrsum += NFSX_V4TIME; - } else { + } else if (!nd->nd_repstat) { vfs_timestamp(&nvap->na_atime); nvap->na_vaflags |= VA_UTIMES_NULL; } @@ -3170,7 +3175,8 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, break; case NFSATTRBIT_TIMECREATE: NFSM_DISSECT(tl, u_int32_t *, NFSX_V4TIME); - fxdr_nfsv4time(tl, &nvap->na_btime); + if (!nd->nd_repstat) + fxdr_nfsv4time(tl, &nvap->na_btime); attrsum += NFSX_V4TIME; break; case NFSATTRBIT_TIMEMODIFYSET: @@ -3178,10 +3184,11 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, attrsum += NFSX_UNSIGNED; if (fxdr_unsigned(int, *tl)==NFSV4SATTRTIME_TOCLIENT) { NFSM_DISSECT(tl, u_int32_t *, NFSX_V4TIME); - fxdr_nfsv4time(tl, &nvap->na_mtime); + if (!nd->nd_repstat) + fxdr_nfsv4time(tl, &nvap->na_mtime); nvap->na_vaflags &= ~VA_UTIMES_NULL; attrsum += NFSX_V4TIME; - } else { + } else if (!nd->nd_repstat) { vfs_timestamp(&nvap->na_mtime); if (!toclient) nvap->na_vaflags |= VA_UTIMES_NULL; @@ -3199,18 +3206,21 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, * specified and this attribute cannot be done in the * same Setattr operation. */ - if ((nd->nd_flag & ND_NFSV41) == 0) - nd->nd_repstat = NFSERR_ATTRNOTSUPP; - else if ((mode & ~07777) != 0 || (mask & ~07777) != 0 || - vp == NULL) - nd->nd_repstat = NFSERR_INVAL; - else if (moderet == 0) - moderet = VOP_GETATTR(vp, &va, nd->nd_cred); - if (moderet == 0) - nvap->na_mode = (mode & mask) | - (va.va_mode & ~mask); - else - nd->nd_repstat = moderet; + if (!nd->nd_repstat) { + if ((nd->nd_flag & ND_NFSV41) == 0) + nd->nd_repstat = NFSERR_ATTRNOTSUPP; + else if ((mode & ~07777) != 0 || + (mask & ~07777) != 0 || vp == NULL) + nd->nd_repstat = NFSERR_INVAL; + else if (moderet == 0) + moderet = VOP_GETATTR(vp, &va, + nd->nd_cred); + if (moderet == 0) + nvap->na_mode = (mode & mask) | + (va.va_mode & ~mask); + else + nd->nd_repstat = moderet; + } attrsum += 2 * NFSX_UNSIGNED; break; default: @@ -3225,7 +3235,7 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap, /* * some clients pad the attrlist, so we need to skip over the - * padding. + * padding. This also skips over unparsed non-supported attributes. */ if (attrsum > attrsize) { error = NFSERR_BADXDR; diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c b/sys/fs/nfsserver/nfs_nfsdserv.c index 47e3a20390f4..a38ef3d47946 100644 --- a/sys/fs/nfsserver/nfs_nfsdserv.c +++ b/sys/fs/nfsserver/nfs_nfsdserv.c @@ -375,6 +375,7 @@ nfsrvd_setattr(struct nfsrv_descript *nd, __unused int isdgram, NFSACL_T *aclp = NULL; struct thread *p = curthread; + NFSZERO_ATTRBIT(&retbits); if (nd->nd_repstat) { nfsrv_wcc(nd, preat_ret, &nva2, postat_ret, &nva); goto out; @@ -402,7 +403,6 @@ nfsrvd_setattr(struct nfsrv_descript *nd, __unused int isdgram, goto nfsmout; /* For NFSv4, only va_uid is used from nva2. */ - NFSZERO_ATTRBIT(&retbits); NFSSETBIT_ATTRBIT(&retbits, NFSATTRBIT_OWNER); preat_ret = nfsvno_getattr(vp, &nva2, nd, p, 1, &retbits); if (!nd->nd_repstat)