From c54d77256c434d51a70e7dbb164ca8ea3f399626 Mon Sep 17 00:00:00 2001 From: Asias He Date: Tue, 19 Aug 2014 09:26:13 +0800 Subject: [PATCH] vfs: Fix race in fdrop There is a race window between f_count reaches 0 and we set f_count to INT_MIN. This race would result in multiple call to fp->close() and rcu_dispose(fp) if we call them in fdrop(). To fix, we set f_count to INT_MIN using CAS, this way, we make sure fp->close() and rcu_dispose(fp) won't be called twice. Fixes #293. Signed-off-by: Asias He --- fs/vfs/kern_descrip.cc | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/fs/vfs/kern_descrip.cc b/fs/vfs/kern_descrip.cc index b8387bd1d8..86a72be177 100644 --- a/fs/vfs/kern_descrip.cc +++ b/fs/vfs/kern_descrip.cc @@ -185,16 +185,25 @@ void fhold(struct file* fp) int fdrop(struct file *fp) { - if (__sync_fetch_and_sub(&fp->f_count, 1) != 1) - return 0; + bool do_free = false; + int o = fp->f_count, n; + do { + n = o - 1; + if (n == 0) { + /* We are about to free this file structure, but we still do things with it + * so set the refcount to INT_MIN, fhold/fdrop may get called again + * and we don't want to reach this point more than once. + * INT_MIN is also safe against fget() seeing this file. + */ + n = INT_MIN; + do_free = true; + } + } while (!__atomic_compare_exchange_n(&fp->f_count, &o, n, true, + __ATOMIC_RELAXED, __ATOMIC_RELAXED)); - /* We are about to free this file structure, but we still do things with it - * so set the refcount to INT_MIN, fhold/fdrop may get called again - * and we don't want to reach this point more than once. - * INT_MIN is also safe against fget() seeing this file. - */ + if (!do_free) + return 0; - fp->f_count = INT_MIN; fp->stop_polls(); fp->close(); rcu_dispose(fp);