From owner-svn-src-stable@freebsd.org Wed Jun 10 03:57:11 2020 Return-Path: Delivered-To: svn-src-stable@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 7E5D334700F; Wed, 10 Jun 2020 03:57:11 +0000 (UTC) (envelope-from freqlabs@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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49hY8l2cQ8z4Z3P; Wed, 10 Jun 2020 03:57:11 +0000 (UTC) (envelope-from freqlabs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5467720CA3; Wed, 10 Jun 2020 03:57:11 +0000 (UTC) (envelope-from freqlabs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 05A3vBhC025626; Wed, 10 Jun 2020 03:57:11 GMT (envelope-from freqlabs@FreeBSD.org) Received: (from freqlabs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 05A3vA4p025622; Wed, 10 Jun 2020 03:57:10 GMT (envelope-from freqlabs@FreeBSD.org) Message-Id: <202006100357.05A3vA4p025622@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: freqlabs set sender to freqlabs@FreeBSD.org using -f From: Ryan Moeller Date: Wed, 10 Jun 2020 03:57:10 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r362001 - in stable/12/sys: fs/tmpfs sys X-SVN-Group: stable-12 X-SVN-Commit-Author: freqlabs X-SVN-Commit-Paths: in stable/12/sys: fs/tmpfs sys X-SVN-Commit-Revision: 362001 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Jun 2020 03:57:11 -0000 Author: freqlabs Date: Wed Jun 10 03:57:10 2020 New Revision: 362001 URL: https://svnweb.freebsd.org/changeset/base/362001 Log: MFC r361748: tmpfs: Preserve alignment of struct fid fields On 64-bit platforms, the two short fields in `struct tmpfs_fid` are padded to the 64-bit alignment of the long field. This pushes the offsets of the subsequent fields by 4 bytes and makes `struct tmpfs_fid` bigger than `struct fid`. `tmpfs_vptofh()` casts a `struct fid *` to `struct tmpfs_fid *`, causing 4 bytes of adjacent memory to be overwritten when the struct fields are set. Through several layers of indirection and embedded structs, the adjacent memory for one particular call to `tmpfs_vptofh()` happens to be the stack canary for `nfsrvd_compound()`. Half of the canary ends up being clobbered, going unnoticed until eventually the stack check fails when `nfsrvd_compound()` returns and a panic is triggered. Instead of duplicating fields of `struct fid` in `struct tmpfs_fid`, narrow the struct to cover only the unique fields for tmpfs and assert at compile time that the struct fits in the allotted space. This way we don't have to replicate the offsets of `struct fid` fields, we just use them directly. Reviewed by: kib, mav, rmacklem Approved by: mav (mentor) Sponsored by: iXsystems, Inc. Differential Revision: https://reviews.freebsd.org/D25077 Modified: stable/12/sys/fs/tmpfs/tmpfs.h stable/12/sys/fs/tmpfs/tmpfs_vfsops.c stable/12/sys/fs/tmpfs/tmpfs_vnops.c stable/12/sys/sys/mount.h Directory Properties: stable/12/ (props changed) Modified: stable/12/sys/fs/tmpfs/tmpfs.h ============================================================================== --- stable/12/sys/fs/tmpfs/tmpfs.h Wed Jun 10 03:36:17 2020 (r362000) +++ stable/12/sys/fs/tmpfs/tmpfs.h Wed Jun 10 03:57:10 2020 (r362001) @@ -37,6 +37,7 @@ #ifndef _FS_TMPFS_TMPFS_H_ #define _FS_TMPFS_TMPFS_H_ +#include #include #include @@ -391,12 +392,12 @@ struct tmpfs_mount { * This structure maps a file identifier to a tmpfs node. Used by the * NFS code. */ -struct tmpfs_fid { - uint16_t tf_len; - uint16_t tf_pad; - ino_t tf_id; - unsigned long tf_gen; +struct tmpfs_fid_data { + ino_t tfd_id; + unsigned long tfd_gen; }; +_Static_assert(sizeof(struct tmpfs_fid_data) <= MAXFIDSZ, + "(struct tmpfs_fid_data) is larger than (struct fid).fid_data"); struct tmpfs_dir_cursor { struct tmpfs_dirent *tdc_current; Modified: stable/12/sys/fs/tmpfs/tmpfs_vfsops.c ============================================================================== --- stable/12/sys/fs/tmpfs/tmpfs_vfsops.c Wed Jun 10 03:36:17 2020 (r362000) +++ stable/12/sys/fs/tmpfs/tmpfs_vfsops.c Wed Jun 10 03:57:10 2020 (r362001) @@ -560,24 +560,29 @@ static int tmpfs_fhtovp(struct mount *mp, struct fid *fhp, int flags, struct vnode **vpp) { - struct tmpfs_fid *tfhp; + struct tmpfs_fid_data tfd; struct tmpfs_mount *tmp; struct tmpfs_node *node; int error; + if (fhp->fid_len != sizeof(tfd)) + return (EINVAL); + + /* + * Copy from fid_data onto the stack to avoid unaligned pointer use. + * See the comment in sys/mount.h on struct fid for details. + */ + memcpy(&tfd, fhp->fid_data, fhp->fid_len); + tmp = VFS_TO_TMPFS(mp); - tfhp = (struct tmpfs_fid *)fhp; - if (tfhp->tf_len != sizeof(struct tmpfs_fid)) + if (tfd.tfd_id >= tmp->tm_nodes_max) return (EINVAL); - if (tfhp->tf_id >= tmp->tm_nodes_max) - return (EINVAL); - TMPFS_LOCK(tmp); LIST_FOREACH(node, &tmp->tm_nodes_used, tn_entries) { - if (node->tn_id == tfhp->tf_id && - node->tn_gen == tfhp->tf_gen) { + if (node->tn_id == tfd.tfd_id && + node->tn_gen == tfd.tfd_gen) { tmpfs_ref_node(node); break; } Modified: stable/12/sys/fs/tmpfs/tmpfs_vnops.c ============================================================================== --- stable/12/sys/fs/tmpfs/tmpfs_vnops.c Wed Jun 10 03:36:17 2020 (r362000) +++ stable/12/sys/fs/tmpfs/tmpfs_vnops.c Wed Jun 10 03:57:10 2020 (r362001) @@ -1410,16 +1410,28 @@ tmpfs_pathconf(struct vop_pathconf_args *v) static int tmpfs_vptofh(struct vop_vptofh_args *ap) +/* +vop_vptofh { + IN struct vnode *a_vp; + IN struct fid *a_fhp; +}; +*/ { - struct tmpfs_fid *tfhp; + struct tmpfs_fid_data tfd; struct tmpfs_node *node; + struct fid *fhp; - tfhp = (struct tmpfs_fid *)ap->a_fhp; node = VP_TO_TMPFS_NODE(ap->a_vp); + fhp = ap->a_fhp; + fhp->fid_len = sizeof(tfd); - tfhp->tf_len = sizeof(struct tmpfs_fid); - tfhp->tf_id = node->tn_id; - tfhp->tf_gen = node->tn_gen; + /* + * Copy into fid_data from the stack to avoid unaligned pointer use. + * See the comment in sys/mount.h on struct fid for details. + */ + tfd.tfd_id = node->tn_id; + tfd.tfd_gen = node->tn_gen; + memcpy(fhp->fid_data, &tfd, fhp->fid_len); return (0); } Modified: stable/12/sys/sys/mount.h ============================================================================== --- stable/12/sys/sys/mount.h Wed Jun 10 03:36:17 2020 (r362000) +++ stable/12/sys/sys/mount.h Wed Jun 10 03:57:10 2020 (r362001) @@ -57,6 +57,9 @@ typedef struct fsid { int32_t val[2]; } fsid_t; /* fil /* * File identifier. * These are unique per filesystem on a single machine. + * + * Note that the offset of fid_data is 4 bytes, so care must be taken to avoid + * undefined behavior accessing unaligned fields within an embedded struct. */ #define MAXFIDSZ 16