From owner-dev-commits-src-branches@freebsd.org Tue May 11 00:28:36 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 1CCC4647DB5; Tue, 11 May 2021 00:28:36 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FfJgS0P3Hz4hHX; Tue, 11 May 2021 00:28:36 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 0062661AE; Tue, 11 May 2021 00:28:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 14B0SZOU077171; Tue, 11 May 2021 00:28:35 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 14B0SZVY077170; Tue, 11 May 2021 00:28:35 GMT (envelope-from git) Date: Tue, 11 May 2021 00:28:35 GMT Message-Id: <202105110028.14B0SZVY077170@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 2bad237ec94f - stable/12 - nfsclient: Copy only initialized fields in nfs_getattr() MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 2bad237ec94f701e3041c146db136f03e6c89324 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 May 2021 00:28:36 -0000 The branch stable/12 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=2bad237ec94f701e3041c146db136f03e6c89324 commit 2bad237ec94f701e3041c146db136f03e6c89324 Author: Mark Johnston AuthorDate: 2021-05-04 12:53:57 +0000 Commit: Mark Johnston CommitDate: 2021-05-11 00:28:22 +0000 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 Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D30090 (cherry picked from commit 8bde6d15d1fa9a947c2bdc5eddae36cfbb1076dc) --- 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 a0181df0bde7..5371f29d00de 100644 --- a/sys/fs/nfs/nfsport.h +++ b/sys/fs/nfs/nfsport.h @@ -880,6 +880,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 e49b276ec66d..f0ac1a0b09ca 100644 --- a/sys/fs/nfsclient/nfs_clcomsubs.c +++ b/sys/fs/nfsclient/nfs_clcomsubs.c @@ -234,6 +234,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 7f6bbe622180..20500a290ff0 100644 --- a/sys/fs/nfsclient/nfs_clport.c +++ b/sys/fs/nfsclient/nfs_clport.c @@ -399,6 +399,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 @@ -550,7 +572,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 fadcf26c686a..17859a16424b 100644 --- a/sys/fs/nfsclient/nfs_clvnops.c +++ b/sys/fs/nfsclient/nfs_clvnops.c @@ -916,23 +916,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.