summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRichard Yao <[email protected]>2015-11-04 16:41:13 -0500
committerBrian Behlendorf <[email protected]>2015-12-03 15:44:47 -0800
commit1683e75edc1fdca1eba53f16ca08ce32c5e736d1 (patch)
treeef22c0fa413442cb31df3bb4b5f37c2745a4d7e9
parentd28c5c4f0472e9674d34f8237ba14187669dad7c (diff)
Fix race between getf() and areleasef()
If a vnode is released asynchronously through areleasef(), it is possible for the user process to reuse the file descriptor before areleasef is called. When this happens, getf() will return a stale reference, any operations in the kernel on that file descriptor will fail (as it is closed) and the operations meant for that fd will never occur from userspace's perspective. We correct this by detecting this condition in getf(), doing a putf on the old file handle, updating the file descriptor and proceeding as if everything was fine. When the areleasef() is done, it will harmlessly decrement the reference counter on the Illumos file handle. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #492
-rw-r--r--include/sys/user.h4
-rw-r--r--module/spl/spl-vnode.c13
2 files changed, 15 insertions, 2 deletions
diff --git a/include/sys/user.h b/include/sys/user.h
index ebbe8f68e..2b25dd33c 100644
--- a/include/sys/user.h
+++ b/include/sys/user.h
@@ -30,8 +30,8 @@
* about the Linux task_struct. Since this is internal to our compatibility
* layer, we make it an opaque type.
*
- * XXX: If the descriptor changes under us, we would get an incorrect
- * reference.
+ * XXX: If the descriptor changes under us and we do not do a getf() between
+ * the change and using it, we would get an incorrect reference.
*/
struct uf_info;
diff --git a/module/spl/spl-vnode.c b/module/spl/spl-vnode.c
index ab9830d18..86349c54a 100644
--- a/module/spl/spl-vnode.c
+++ b/module/spl/spl-vnode.c
@@ -656,6 +656,19 @@ vn_getf(int fd)
fp = file_find(fd, current);
if (fp) {
+ lfp = fget(fd);
+ fput(fp->f_file);
+ /*
+ * areleasef() can cause us to see a stale reference when
+ * userspace has reused a file descriptor before areleasef()
+ * has run. fput() the stale reference and replace it. We
+ * retain the original reference count such that the concurrent
+ * areleasef() will decrement its reference and terminate.
+ */
+ if (lfp != fp->f_file) {
+ fp->f_file = lfp;
+ fp->f_vnode->v_file = lfp;
+ }
atomic_inc(&fp->f_ref);
spin_unlock(&vn_file_lock);
return (fp);