Skip to content

Commit

Permalink
Fix race between getf() and areleasef()
Browse files Browse the repository at this point in the history
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 <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #492
  • Loading branch information
ryao authored and behlendorf committed Dec 3, 2015
1 parent d28c5c4 commit 1683e75
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
4 changes: 2 additions & 2 deletions include/sys/user.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions module/spl/spl-vnode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 1683e75

Please sign in to comment.