hardenedbsd/HardenedBSD
1
0
Fork 0
mirror of https://git.hardenedbsd.org/hardenedbsd/HardenedBSD.git synced 2024-11-26 10:53:39 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

nfsd: Fix handling of NFSv4 setable attributes

Browse Source 
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.

MFC after:	 2 weeks
This commit is contained in:
Rick Macklem 2024-08-27 14:19:33 -07:00
parent fe66e4caf4
commit 5037c6398b
2 changed files with 44 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
sys/fs/nfsserver/nfs_nfsdport.c
View File

@ -3026,6 +3026,8 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
/* /*
* Loop around getting the setable attributes. If an unsupported * Loop around getting the setable attributes. If an unsupported
* one is found, set nd_repstat == NFSERR_ATTRNOTSUPP and return. * 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) { if (retnotsup) {
nd->nd_repstat = NFSERR_ATTRNOTSUPP; nd->nd_repstat = NFSERR_ATTRNOTSUPP;
@ -3043,12 +3045,13 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
switch (bitpos) { switch (bitpos) {
case NFSATTRBIT_SIZE: case NFSATTRBIT_SIZE:
NFSM_DISSECT(tl, u_int32_t *, NFSX_HYPER); NFSM_DISSECT(tl, u_int32_t *, NFSX_HYPER);
if (vp != NULL && vp->v_type != VREG) { if (!nd->nd_repstat) {
error = (vp->v_type == VDIR) ? NFSERR_ISDIR : if (vp != NULL && vp->v_type != VREG)
NFSERR_INVAL; nd->nd_repstat = (vp->v_type == VDIR) ?
goto nfsmout; NFSERR_ISDIR : NFSERR_INVAL;
else
nvap->na_size = fxdr_hyper(tl);
} }
nvap->na_size = fxdr_hyper(tl);
attrsum += NFSX_HYPER; attrsum += NFSX_HYPER;
break; break;
case NFSATTRBIT_ACL: case NFSATTRBIT_ACL:
@ -3085,7 +3088,8 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
case NFSATTRBIT_MODE: case NFSATTRBIT_MODE:
moderet = NFSERR_INVAL; /* Can't do MODESETMASKED. */ moderet = NFSERR_INVAL; /* Can't do MODESETMASKED. */
NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); 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; attrsum += NFSX_UNSIGNED;
break; break;
case NFSATTRBIT_OWNER: case NFSATTRBIT_OWNER:
@ -3153,10 +3157,11 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
attrsum += NFSX_UNSIGNED; attrsum += NFSX_UNSIGNED;
if (fxdr_unsigned(int, *tl)==NFSV4SATTRTIME_TOCLIENT) { if (fxdr_unsigned(int, *tl)==NFSV4SATTRTIME_TOCLIENT) {
NFSM_DISSECT(tl, u_int32_t *, NFSX_V4TIME); 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; toclient = 1;
attrsum += NFSX_V4TIME; attrsum += NFSX_V4TIME;
} else { } else if (!nd->nd_repstat) {
vfs_timestamp(&nvap->na_atime); vfs_timestamp(&nvap->na_atime);
nvap->na_vaflags |= VA_UTIMES_NULL; nvap->na_vaflags |= VA_UTIMES_NULL;
} }
@ -3169,7 +3174,8 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
break; break;
case NFSATTRBIT_TIMECREATE: case NFSATTRBIT_TIMECREATE:
NFSM_DISSECT(tl, u_int32_t *, NFSX_V4TIME); 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; attrsum += NFSX_V4TIME;
break; break;
case NFSATTRBIT_TIMEMODIFYSET: case NFSATTRBIT_TIMEMODIFYSET:
@ -3177,10 +3183,11 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
attrsum += NFSX_UNSIGNED; attrsum += NFSX_UNSIGNED;
if (fxdr_unsigned(int, *tl)==NFSV4SATTRTIME_TOCLIENT) { if (fxdr_unsigned(int, *tl)==NFSV4SATTRTIME_TOCLIENT) {
NFSM_DISSECT(tl, u_int32_t *, NFSX_V4TIME); 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; nvap->na_vaflags &= ~VA_UTIMES_NULL;
attrsum += NFSX_V4TIME; attrsum += NFSX_V4TIME;
} else { } else if (!nd->nd_repstat) {
vfs_timestamp(&nvap->na_mtime); vfs_timestamp(&nvap->na_mtime);
if (!toclient) if (!toclient)
nvap->na_vaflags |= VA_UTIMES_NULL; nvap->na_vaflags |= VA_UTIMES_NULL;
@ -3198,18 +3205,21 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
* specified and this attribute cannot be done in the * specified and this attribute cannot be done in the
* same Setattr operation. * same Setattr operation.
*/ */
if ((nd->nd_flag & ND_NFSV41) == 0) if (!nd->nd_repstat) {
nd->nd_repstat = NFSERR_ATTRNOTSUPP; if ((nd->nd_flag & ND_NFSV41) == 0)
else if ((mode & ~07777) != 0 || (mask & ~07777) != 0 || nd->nd_repstat = NFSERR_ATTRNOTSUPP;
vp == NULL) else if ((mode & ~07777) != 0 ||
nd->nd_repstat = NFSERR_INVAL; (mask & ~07777) != 0 || vp == NULL)
else if (moderet == 0) nd->nd_repstat = NFSERR_INVAL;
moderet = VOP_GETATTR(vp, &va, nd->nd_cred); else if (moderet == 0)
if (moderet == 0) moderet = VOP_GETATTR(vp, &va,
nvap->na_mode = (mode & mask) | nd->nd_cred);
(va.va_mode & ~mask); if (moderet == 0)
else nvap->na_mode = (mode & mask) |
nd->nd_repstat = moderet; (va.va_mode & ~mask);
else
nd->nd_repstat = moderet;
}
attrsum += 2 * NFSX_UNSIGNED; attrsum += 2 * NFSX_UNSIGNED;
break; break;
case NFSATTRBIT_MODEUMASK: case NFSATTRBIT_MODEUMASK:
@ -3220,13 +3230,15 @@ nfsv4_sattr(struct nfsrv_descript *nd, vnode_t vp, struct nfsvattr *nvap,
* If moderet != 0, mode has already been done. * If moderet != 0, mode has already been done.
* If vp != NULL, this is not a file object creation. * If vp != NULL, this is not a file object creation.
*/ */
if ((nd->nd_flag & ND_NFSV42) == 0) if (!nd->nd_repstat) {
nd->nd_repstat = NFSERR_ATTRNOTSUPP; if ((nd->nd_flag & ND_NFSV42) == 0)
else if ((mask & ~0777) != 0 || vp != NULL || nd->nd_repstat = NFSERR_ATTRNOTSUPP;
moderet != 0) else if ((mask & ~0777) != 0 || vp != NULL ||
nd->nd_repstat = NFSERR_INVAL; moderet != 0)
else nd->nd_repstat = NFSERR_INVAL;
nvap->na_mode = (mode & ~mask); else
nvap->na_mode = (mode & ~mask);
}
attrsum += 2 * NFSX_UNSIGNED; attrsum += 2 * NFSX_UNSIGNED;
break; break;
default: default:
@ -3241,7 +3253,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 * 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) { if (attrsum > attrsize) {
error = NFSERR_BADXDR; error = NFSERR_BADXDR;

2
sys/fs/nfsserver/nfs_nfsdserv.c
View File

@ -375,6 +375,7 @@ nfsrvd_setattr(struct nfsrv_descript *nd, __unused int isdgram,
NFSACL_T *aclp = NULL; NFSACL_T *aclp = NULL;
struct thread *p = curthread; struct thread *p = curthread;
NFSZERO_ATTRBIT(&retbits);
if (nd->nd_repstat) { if (nd->nd_repstat) {
nfsrv_wcc(nd, preat_ret, &nva2, postat_ret, &nva); nfsrv_wcc(nd, preat_ret, &nva2, postat_ret, &nva);
goto out; goto out;
@ -402,7 +403,6 @@ nfsrvd_setattr(struct nfsrv_descript *nd, __unused int isdgram,
goto nfsmout; goto nfsmout;
/* For NFSv4, only va_uid is used from nva2. */ /* For NFSv4, only va_uid is used from nva2. */
NFSZERO_ATTRBIT(&retbits);
NFSSETBIT_ATTRBIT(&retbits, NFSATTRBIT_OWNER); NFSSETBIT_ATTRBIT(&retbits, NFSATTRBIT_OWNER);
preat_ret = nfsvno_getattr(vp, &nva2, nd, p, 1, &retbits); preat_ret = nfsvno_getattr(vp, &nva2, nd, p, 1, &retbits);
if (!nd->nd_repstat) if (!nd->nd_repstat)