From b37a7c00afef1d167a3d2e2de799d16d733854f2 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Fri, 3 Jul 2015 21:55:10 -0700 Subject: [PATCH] tmpfs - Fix write-append/mmap-read race * tmpfs_write() was not extending the target file atomically with the write via a file node lock. Although the vnode is locked exclusively, this only prevents read() and write() related races. lseek() and memory mapped reads could still race. * Adjust the NODE LOCK to cover the entire tmpfs_write() operation. This fixes races against lseek() or stat() which would otherwise allow the new post-append file size to be returned before the write completes. -Matt Reported-by: Ben Woolley --- sys/vfs/tmpfs/tmpfs.h | 4 ++-- sys/vfs/tmpfs/tmpfs_subr.c | 21 ++++++++------------- sys/vfs/tmpfs/tmpfs_vnops.c | 23 +++++++++++++---------- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/sys/vfs/tmpfs/tmpfs.h b/sys/vfs/tmpfs/tmpfs.h index ad29b1195e..db09fd9be6 100644 --- a/sys/vfs/tmpfs/tmpfs.h +++ b/sys/vfs/tmpfs/tmpfs.h @@ -312,11 +312,11 @@ struct tmpfs_mount { * used directly as it may be bigger than the current amount of * free memory; in the extreme case, it will hold the SIZE_MAX * value. Instead, use the TMPFS_PAGES_MAX macro. */ - vm_pindex_t tm_pages_max; + long tm_pages_max; /* Number of pages in use by the file system. Cannot be bigger * than the value returned by TMPFS_PAGES_MAX in any case. */ - vm_pindex_t tm_pages_used; + long tm_pages_used; /* Pointer to the node representing the root directory of this * file system. */ diff --git a/sys/vfs/tmpfs/tmpfs_subr.c b/sys/vfs/tmpfs/tmpfs_subr.c index 878886b735..dd1aa658f3 100644 --- a/sys/vfs/tmpfs/tmpfs_subr.c +++ b/sys/vfs/tmpfs/tmpfs_subr.c @@ -277,9 +277,7 @@ tmpfs_free_node(struct tmpfs_mount *tmp, struct tmpfs_node *node) objcache_put(tmp->tm_node_pool, node); /* node is now invalid */ - TMPFS_LOCK(tmp); - tmp->tm_pages_used -= pages; - TMPFS_UNLOCK(tmp); + atomic_add_long(&tmp->tm_pages_used, -(long)pages); } /* --------------------------------------------------------------------- */ @@ -949,6 +947,8 @@ tmpfs_dir_getdents(struct tmpfs_node *node, struct uio *uio, off_t *cntp) * to be zero filled. * * Returns zero on success or an appropriate error code on failure. + * + * Caller must hold the node exclusively locked. */ int tmpfs_reg_resize(struct vnode *vp, off_t newsize, int trivial) @@ -973,7 +973,6 @@ tmpfs_reg_resize(struct vnode *vp, off_t newsize, int trivial) * because the last allocated page can accommodate the change on * its own. */ - TMPFS_NODE_LOCK(node); oldsize = node->tn_size; oldpages = round_page64(oldsize) / PAGE_SIZE; KKASSERT(oldpages == node->tn_reg.tn_aobj_pages); @@ -981,17 +980,13 @@ tmpfs_reg_resize(struct vnode *vp, off_t newsize, int trivial) if (newpages > oldpages && tmp->tm_pages_used + newpages - oldpages > tmp->tm_pages_max) { - TMPFS_NODE_UNLOCK(node); error = ENOSPC; goto out; } node->tn_reg.tn_aobj_pages = newpages; node->tn_size = newsize; - TMPFS_NODE_UNLOCK(node); - TMPFS_LOCK(tmp); - tmp->tm_pages_used += (newpages - oldpages); - TMPFS_UNLOCK(tmp); + atomic_add_long(&tmp->tm_pages_used, (newpages - oldpages)); /* * When adjusting the vnode filesize and its VM object we must @@ -1311,6 +1306,9 @@ tmpfs_update(struct vnode *vp) /* --------------------------------------------------------------------- */ +/* + * Caller must hold an exclusive node lock. + */ int tmpfs_truncate(struct vnode *vp, off_t length) { @@ -1335,11 +1333,8 @@ tmpfs_truncate(struct vnode *vp, off_t length) error = tmpfs_reg_resize(vp, length, 1); - if (error == 0) { - TMPFS_NODE_LOCK(node); + if (error == 0) node->tn_status |= TMPFS_NODE_CHANGED | TMPFS_NODE_MODIFIED; - TMPFS_NODE_UNLOCK(node); - } out: tmpfs_update(vp); diff --git a/sys/vfs/tmpfs/tmpfs_vnops.c b/sys/vfs/tmpfs/tmpfs_vnops.c index 86c3185103..bbeb2183a9 100644 --- a/sys/vfs/tmpfs/tmpfs_vnops.c +++ b/sys/vfs/tmpfs/tmpfs_vnops.c @@ -572,6 +572,8 @@ tmpfs_write (struct vop_write_args *ap) return (EINVAL); seqcount = ap->a_ioflag >> 16; + TMPFS_NODE_LOCK(node); + oldsize = node->tn_size; if (ap->a_ioflag & IO_APPEND) uio->uio_offset = node->tn_size; @@ -581,7 +583,8 @@ tmpfs_write (struct vop_write_args *ap) */ if (uio->uio_offset + uio->uio_resid > VFS_TO_TMPFS(vp->v_mount)->tm_maxfilesize) { - return (EFBIG); + error = EFBIG; + goto done; } /* @@ -589,16 +592,15 @@ tmpfs_write (struct vop_write_args *ap) */ if (vp->v_type == VREG && td != NULL && td->td_lwp != NULL) { error = kern_getrlimit(RLIMIT_FSIZE, &limit); - if (error != 0) { - return error; - } + if (error) + goto done; if (uio->uio_offset + uio->uio_resid > limit.rlim_cur) { ksignal(td->td_proc, SIGXFSZ); - return (EFBIG); + error = EFBIG; + goto done; } } - /* * Extend the file's size if necessary */ @@ -625,7 +627,8 @@ tmpfs_write (struct vop_write_args *ap) if ((uio->uio_offset + len) > node->tn_size) { trivial = (uio->uio_offset <= node->tn_size); - error = tmpfs_reg_resize(vp, uio->uio_offset + len, trivial); + error = tmpfs_reg_resize(vp, uio->uio_offset + len, + trivial); if (error) break; } @@ -745,7 +748,6 @@ tmpfs_write (struct vop_write_args *ap) * order to be able to dispose of the buffer cache buffer without * flushing it. */ - TMPFS_NODE_LOCK(node); if (uio->uio_segflg != UIO_NOCOPY) node->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_MODIFIED; if (extended) @@ -755,9 +757,10 @@ tmpfs_write (struct vop_write_args *ap) if (priv_check_cred(ap->a_cred, PRIV_VFS_RETAINSUGID, 0)) node->tn_mode &= ~(S_ISUID | S_ISGID); } - TMPFS_NODE_UNLOCK(node); done: - tmpfs_knote(vp, kflags); + TMPFS_NODE_UNLOCK(node); + if (kflags) + tmpfs_knote(vp, kflags); return(error); } -- 2.41.0