2006-05-16 19:39:43

by Amy Griffis

Subject: [PATCH] fix race in inotify_release

While doing some inotify stress testing, I hit the following race. In
inotify_release(), it's possible for a watch to be removed from the
lists in between dropping dev->mutex and taking inode->inotify_mutex.
The reference we hold prevents the watch from being freed, but not
from being removed.

Checking the dev's idr mapping will prevent a double list_del of the
same watch.

Signed-off-by: Amy Griffis <[email protected]>

diff --git a/fs/inotify.c b/fs/inotify.c
index 1f50302..7d57253 100644
--- a/fs/inotify.c
+++ b/fs/inotify.c
@@ -848,7 +848,11 @@ static int inotify_release(struct inode
inode = watch->inode;
mutex_lock(&inode->inotify_mutex);
mutex_lock(&dev->mutex);
- remove_watch_no_event(watch, dev);
+
+ /* make sure we didn't race with another list removal */
+ if (likely(idr_find(&dev->idr, watch->wd)))
+ remove_watch_no_event(watch, dev);
+
mutex_unlock(&dev->mutex);
mutex_unlock(&inode->inotify_mutex);
put_inotify_watch(watch);

2006-05-16 19:54:36

by John McCutchan

Subject: Re: [PATCH] fix race in inotify_release

On Tue, 2006-05-16 at 15:39 -0400, Amy Griffis wrote:
> While doing some inotify stress testing, I hit the following race. In
> inotify_release(), it's possible for a watch to be removed from the
> lists in between dropping dev->mutex and taking inode->inotify_mutex.
> The reference we hold prevents the watch from being freed, but not
> from being removed.
>
> Checking the dev's idr mapping will prevent a double list_del of the
> same watch.

Looks good. Thanks for the patch and stress testing inotify!


--
John McCutchan <[email protected]>