From 8bde6d15d1fa9a947c2bdc5eddae36cfbb1076dc Mon Sep 17 00:00:00 2001
From: Mark Johnston <markj@FreeBSD.org>
Date: Tue, 4 May 2021 08:53:57 -0400
Subject: [PATCH] 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
---
 sys/fs/nfs/nfsport.h             |  1 +
 sys/fs/nfsclient/nfs_clcomsubs.c |  1 +
 sys/fs/nfsclient/nfs_clport.c    | 24 +++++++++++++++++++++++-
 sys/fs/nfsclient/nfs_clvnops.c   | 19 ++-----------------
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/sys/fs/nfs/nfsport.h b/sys/fs/nfs/nfsport.h
index 6777dc72f6a..cb82666ab39 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 6a36aed478d..8a51d51f093 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 81a0e05c323..64820cd11f1 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 217290b080b..5f81bb5b42a 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.