d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nfsclient: Copy only initialized fields in nfs_getattr()

Browse Source 
When loading attributes from the cache, the NFS client is careful to
copy only the fields that it initialized.  After fetching attributes
from the server, however, it would copy the entire vattr structure
initialized from the RPC response, so uninitialized stack bytes would
end up being copied to userspace.  In particular, va_birthtime (v2 and
v3) and va_gen (v3) had this problem.

Use a common subroutine to copy fields provided by the NFS client, and
ensure that we provide a dummy va_gen for the v3 case.

Reviewed by:	rmacklem
Reported by:	KMSAN
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D30090
This commit is contained in:
Mark Johnston 2021-05-04 08:53:57 -04:00
parent ee384b229d
commit 8bde6d15d1
4 changed files with 27 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sys/fs/nfs/nfsport.h
View File

@ -1001,6 +1001,7 @@ int nfscl_loadattrcache(struct vnode **, struct nfsvattr *, void *, void *,
int, int);
int newnfs_realign(struct mbuf **, int);
bool ncl_pager_setsize(struct vnode *vp, u_quad_t *nsizep);
void ncl_copy_vattr(struct vattr *dst, struct vattr *src);
/*
* If the port runs on an SMP box that can enforce Atomic ops with low

1
sys/fs/nfsclient/nfs_clcomsubs.c
View File

@ -285,6 +285,7 @@ nfsm_loadattr(struct nfsrv_descript *nd, struct nfsvattr *nap)
fxdr_nfsv3time(&fp->fa3_ctime, &nap->na_ctime);
fxdr_nfsv3time(&fp->fa3_mtime, &nap->na_mtime);
nap->na_flags = 0;
nap->na_gen = 0;
nap->na_filerev = 0;
} else {
NFSM_DISSECT(fp, struct nfs_fattr *, NFSX_V2FATTR);

24
sys/fs/nfsclient/nfs_clport.c
View File

@ -400,6 +400,28 @@ nfscl_warn_fileid(struct nfsmount *nmp, struct nfsvattr *oldnap,
ncl_fileid_maxwarnings);
}
void
ncl_copy_vattr(struct vattr *dst, struct vattr *src)
{
dst->va_type = src->va_type;
dst->va_mode = src->va_mode;
dst->va_nlink = src->va_nlink;
dst->va_uid = src->va_uid;
dst->va_gid = src->va_gid;
dst->va_fsid = src->va_fsid;
dst->va_fileid = src->va_fileid;
dst->va_size = src->va_size;
dst->va_blocksize = src->va_blocksize;
dst->va_atime = src->va_atime;
dst->va_mtime = src->va_mtime;
dst->va_ctime = src->va_ctime;
dst->va_gen = src->va_gen;
dst->va_flags = src->va_flags;
dst->va_rdev = src->va_rdev;
dst->va_bytes = src->va_bytes;
dst->va_filerev = src->va_filerev;
}
/*
* Load the attribute cache (that lives in the nfsnode entry) with
* the attributes of the second argument and
@ -551,7 +573,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp);
}
if (vaper != NULL) {
NFSBCOPY((caddr_t)vap, (caddr_t)vaper, sizeof(*vap));
ncl_copy_vattr(vaper, vap);
if (np->n_flag & NCHG) {
if (np->n_flag & NACC)
vaper->va_atime = np->n_atim;

19
sys/fs/nfsclient/nfs_clvnops.c
View File

@ -962,23 +962,8 @@ nfs_getattr(struct vop_getattr_args *ap)
* First look in the cache.
*/
if (ncl_getattrcache(vp, &vattr) == 0) {
vap->va_type = vattr.va_type;
vap->va_mode = vattr.va_mode;
vap->va_nlink = vattr.va_nlink;
vap->va_uid = vattr.va_uid;
vap->va_gid = vattr.va_gid;
vap->va_fsid = vattr.va_fsid;
vap->va_fileid = vattr.va_fileid;
vap->va_size = vattr.va_size;
vap->va_blocksize = vattr.va_blocksize;
vap->va_atime = vattr.va_atime;
vap->va_mtime = vattr.va_mtime;
vap->va_ctime = vattr.va_ctime;
vap->va_gen = vattr.va_gen;
vap->va_flags = vattr.va_flags;
vap->va_rdev = vattr.va_rdev;
vap->va_bytes = vattr.va_bytes;
vap->va_filerev = vattr.va_filerev;
ncl_copy_vattr(vap, &vattr);
/*
* Get the local modify time for the case of a write
* delegation.