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)
MFC after:	1 week
Sponsored by:	iXsystems, Inc.
Differential Revision:	https://reviews.freebsd.org/D25077
This commit is contained in:
Ryan Moeller 2020-06-03 09:38:51 +00:00
parent 06f6997eb5
commit 693d10a291
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=361748
4 changed files with 39 additions and 18 deletions

View File

@ -37,6 +37,7 @@
#ifndef _FS_TMPFS_TMPFS_H_
#define _FS_TMPFS_TMPFS_H_
#include <sys/cdefs.h>
#include <sys/queue.h>
#include <sys/tree.h>
@ -393,12 +394,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;

View File

@ -566,24 +566,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;
tmp = VFS_TO_TMPFS(mp);
tfhp = (struct tmpfs_fid *)fhp;
if (tfhp->tf_len != sizeof(struct tmpfs_fid))
if (fhp->fid_len != sizeof(tfd))
return (EINVAL);
if (tfhp->tf_id >= tmp->tm_nodes_max)
/*
* 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);
if (tfd.tfd_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;
}

View File

@ -1435,16 +1435,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);
}

View File

@ -57,6 +57,9 @@ typedef struct fsid { int32_t val[2]; } fsid_t; /* filesystem id type */
/*
* 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