tmpfs - Fix write-append/mmap-read race
authorMatthew Dillon <dillon@apollo.backplane.com>
Sat, 4 Jul 2015 04:55:10 +0000 (21:55 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sat, 4 Jul 2015 04:55:10 +0000 (21:55 -0700)
* 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 <tautolog@gmail.com>
sys/vfs/tmpfs/tmpfs.h
sys/vfs/tmpfs/tmpfs_subr.c
sys/vfs/tmpfs/tmpfs_vnops.c

index ad29b11..db09fd9 100644 (file)
@@ -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. */
index 878886b..dd1aa65 100644 (file)
@@ -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);
index 86c3185..bbeb218 100644 (file)
@@ -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);
 }