aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Johnston <markj@FreeBSD.org>2021-05-04 12:53:57 +0000
committerMark Johnston <markj@FreeBSD.org>2021-05-04 12:53:57 +0000
commit8bde6d15d1fa9a947c2bdc5eddae36cfbb1076dc (patch)
treeb830dbd27f47e5eb5409971f8a2f7a016b012cdf
parentee384b229dc62b2b0c9180db8c71fa99f30144cf (diff)
downloadsrc-8bde6d15d1fa9a947c2bdc5eddae36cfbb1076dc.tar.gz
src-8bde6d15d1fa9a947c2bdc5eddae36cfbb1076dc.zip
nfsclient: Copy only initialized fields in nfs_getattr()
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
-rw-r--r--sys/fs/nfs/nfsport.h1
-rw-r--r--sys/fs/nfsclient/nfs_clcomsubs.c1
-rw-r--r--sys/fs/nfsclient/nfs_clport.c24
-rw-r--r--sys/fs/nfsclient/nfs_clvnops.c19
4 files changed, 27 insertions, 18 deletions
diff --git a/sys/fs/nfs/nfsport.h b/sys/fs/nfs/nfsport.h
index 6777dc72f6a3..cb82666ab397 100644
--- a/sys/fs/nfs/nfsport.h
+++ b/sys/fs/nfs/nfsport.h
@@ -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
diff --git a/sys/fs/nfsclient/nfs_clcomsubs.c b/sys/fs/nfsclient/nfs_clcomsubs.c
index 6a36aed478d9..8a51d51f093f 100644
--- a/sys/fs/nfsclient/nfs_clcomsubs.c
+++ b/sys/fs/nfsclient/nfs_clcomsubs.c
@@ -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);
diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c
index 81a0e05c3234..64820cd11f1c 100644
--- a/sys/fs/nfsclient/nfs_clport.c
+++ b/sys/fs/nfsclient/nfs_clport.c
@@ -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;
diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
index 217290b080b3..5f81bb5b42a4 100644
--- a/sys/fs/nfsclient/nfs_clvnops.c
+++ b/sys/fs/nfsclient/nfs_clvnops.c
@@ -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.