From nobody Tue Sep 10 01:38:28 2024 X-Original-To: dev-commits-src-all@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 4X2mYw4Qdmz5WCQN; Tue, 10 Sep 2024 01:38:28 +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 4X2mYw3Y6Bz4gsQ; Tue, 10 Sep 2024 01:38:28 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1725932308; 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=KJFkXD6YzuZOCJ+4Lkz9KCvcMhQHhxTL0GbmhJUXpNM=; b=xYFPWTc4Baln0v8TISQO6zLnc5Kirc0Qx0BnK+0B4tFlxhXg7ICbVCTFLCvyT795RNgGVe q1AfkS0wyZFKUDolR8hk0UANzmQjpnNUkT44WkGTHgRUEnwcMCRFCnfC2QzFisUg3w8KQ1 +V9CHm9FZK2ZpWcNl01xG92mBBVscbWbGw1nl2isDAwgvNdeE3OD843oUDZeVa/brC6cy+ HC64DWUIfH8IQKKPpEnOsq2l4+AO6eq38EYp5XTkRWMp+6G1q35YPvYAy+6w8MZYBf3wSf sv9FmpWB95M2YNTGMd9DTV/eWYu54M8LogiZG5hYAvHnjoawIb8Yz3eWxdyPEA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1725932308; a=rsa-sha256; cv=none; b=n9olDiFRqnkWwUbBzzdJ0yaC+ylJbQajM1GAcKPRKddgUJouYdxM3+2LDipKie87HeJGhi 57F1ohKUwnyhi6YS7WsVvzwKnZKbUYL9X5I04f+7q7UHPR2W7KNk+JwFAg7kwgwBtpEp7D L+56MkrsQ5/EQPP3Km0B7xYbiHHyBdmRNyqLiI2ulvRdQTzLKKrzPXhirfrO9pfCNxPV9w WDus+nfLa2K4T/yxYMwywd8LDMDcr6X52tvsCkDWA+XkAx5MKihWFfIVFw4pLxE4fT9t0g e/g0Fq9bMDyeZ++8xwnXpwY/tlv1cEOexuUayvocntGPYJvAPNAJxsOZD/hzOA== 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=1725932308; 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=KJFkXD6YzuZOCJ+4Lkz9KCvcMhQHhxTL0GbmhJUXpNM=; b=sjmerGGMFL8vA3qppUvs2BJwI3xJ4LI2F8BZS454WzjNRXWLlTzRcg6ubHRmwyx43DMQUD o3YSJ3IBBCQZIlgB83MgXpWIJ1M9iIgGGQkAnwjV7ETCLFAO9VCRBm1lSO43xvvjNsQjuq 0kg8ujZVMiGWu3DomZ0C9GG/XGqZTpTvg3N3UdLoJQVqn8TNhnx7PR8GCUGHDt5ul62JXr 5M3rOJCBvveQKHZUpthhZAq1Olin9ibBtUz6Ep6M6G2OvZ9u6CwmqHEGSSiBO6FNnS3jm0 H7Y9f/hsaiZySn4SfUuiTdl3sZTRnR7W6xzsMh+hyKlYUMhpsfDSLJFS/oOr5A== 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 4X2mYw38DDzRkf; Tue, 10 Sep 2024 01:38:28 +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 48A1cSYl013940; Tue, 10 Sep 2024 01:38:28 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 48A1cSMT013937; Tue, 10 Sep 2024 01:38:28 GMT (envelope-from git) Date: Tue, 10 Sep 2024 01:38:28 GMT Message-Id: <202409100138.48A1cSMT013937@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: d72ace979924 - stable/13 - nfsd: Fix handling of NFSv4 setable attributes List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@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/13 X-Git-Reftype: branch X-Git-Commit: d72ace9799240987458cc9bd4c17ac1f72a5109d Auto-Submitted: auto-generated The branch stable/13 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=d72ace9799240987458cc9bd4c17ac1f72a5109d commit d72ace9799240987458cc9bd4c17ac1f72a5109d Author: Rick Macklem AuthorDate: 2024-08-27 21:19:33 +0000 Commit: Rick Macklem CommitDate: 2024-09-10 01:37:24 +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 f412451e4e35..41443a5a79f4 100644 --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -3038,6 +3038,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; @@ -3055,12 +3057,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: @@ -3097,7 +3100,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: @@ -3165,10 +3169,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; } @@ -3181,7 +3186,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: @@ -3189,10 +3195,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; @@ -3210,18 +3217,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: @@ -3236,7 +3246,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 ccaeb4153253..7f7ff9f75a91 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)