[PATCH 1/2] inotify: do not reuse watch descriptors

[PATCH 1/2] inotify: do not reuse watch descriptors

[eparis]

From: Eric Paris <eparis@redhat.com>

Since commit 7e790dd5fc937bc8d2400c30a05e32a9e9eef276 inotify changed the
manor in which it gave watch descriptors back to userspace.  Previous to
this commit inotify acted like the following:

inotify_add_watch(X, Y, Z) = 1
inotify_rm_watch(X, 1);
inotify_add_watch(X, Y, Z) = 2

but after this patch inotify would return watch descriptors like so:

inotify_add_watch(X, Y, Z) = 1
inotify_rm_watch(X, 1);
inotify_add_watch(X, Y, Z) = 1

which I saw as equivalent to opening an fd where

open(file) = 1;
close(1);
open(file) = 1;

seemed perfectly reasonable.  The issue is that quite a bit of userspace
apparently relies on the behavior in which watch descriptors will not be
quickly reused.  KDE relies on it, I know some selinux packages rely on it,
and I have heard complaints from other random sources such as debian bug
558981.

Although the man page implies what we do is ok, we broke userspace so this
patch almost reverts us to the old behavior.  It is still slightly racey
and I have patches that would fix that, but they are rather large and this
will fix it for all real world cases.  The race is as follows:

task1 creates a watch and blocks in idr_new_watch() before it updates the hint.
task2 creates a watch and updates the hint.
task1 updates the hint with it's older wd
task removes the watch created by task2
task adds a new watch and will reuse the wd originally given to task2

it requires moving some locking around the hint (last_wd) but this should
solve it for the real world and be -stable safe.

As a side effect this patch papers over a bug in the lib/idr code which is
causing a large number WARN's to pop on people's system and many reports
in kerneloops.org.  I'm working on the root cause of that idr bug seperately
but this should make inotify immune to that issue.

Signed-off-by: Eric Paris <eparis@redhat.com>
---
 fs/notify/inotify/inotify_user.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 8271cf0..a94e8bd 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -552,7 +552,7 @@ retry:
 
 	spin_lock(&group->inotify_data.idr_lock);
 	ret = idr_get_new_above(&group->inotify_data.idr, &tmp_ientry->fsn_entry,
-				group->inotify_data.last_wd,
+				group->inotify_data.last_wd+1,
 				&tmp_ientry->wd);
 	spin_unlock(&group->inotify_data.idr_lock);
 	if (ret) {
@@ -632,7 +632,7 @@ static struct fsnotify_group *inotify_new_group(struct user_struct *user, unsign
 
 	spin_lock_init(&group->inotify_data.idr_lock);
 	idr_init(&group->inotify_data.idr);
-	group->inotify_data.last_wd = 1;
+	group->inotify_data.last_wd = 0;
 	group->inotify_data.user = user;
 	group->inotify_data.fa = NULL;
 
-- 
1.6.5.3

[PATCH 2/2] inotify: only warn once for inotify problems

[eparis]

From: Eric Paris <eparis@redhat.com>

inotify will WARN() if it finds that the idr and the fsnotify internals
somehow got out of sync.  It was only supposed to do this once but due
to this stupid bug it would warn every single time a problem was detected.

Signed-off-by: Eric Paris <eparis@redhat.com>
---
 fs/notify/inotify/inotify_fsnotify.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index c9ee67b..1afb0a1 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -121,7 +121,7 @@ static int idr_callback(int id, void *p, void *data)
 	if (warned)
 		return 0;
 
-	warned = false;
+	warned = true;
 	entry = p;
 	ientry = container_of(entry, struct inotify_inode_mark_entry, fsn_entry);
 
-- 
1.6.5.3