[PATCH 14/17] fsnotify: detach the connector before destroying its marks

[Christian Brauner <brauner@kernel.org>]

fsnotify_destroy_marks() removes every mark of an object that goes away.
It walks the mark list of the connector and has to drop the connector
lock around fsnotify_destroy_mark() since that sleeps. Afterwards it
continues from the mark it just destroyed. That mark stays on the list
because the function holds a reference to it. But while the lock is
dropped fsnotify_add_mark_list() can add a mark to the very same
connector and put it in front of the current one when group priority
dictates. fsnotify_grab_connector() hands out the connector until it is
detached and it only gets detached after the walk.

So the walk never sees that mark. The connector is detached from the
object with the new mark still attached to it. It never receives an
event nor IN_IGNORED. For inotify that's a watch descriptor that never
reports anything and doesn't show up in fdinfo either:

  wd1 = inotify_add_watch(/proc/self/fd/6) = 2
  events after the unlink:
    wd 1 mask 0x400 IN_DELETE_SELF
    wd 1 mask 0x8000 IN_IGNORED
  events after write() to the file:
    (none)

Detach the connector from the object before the marks are destroyed.
fsnotify_grab_connector() refuses a detached connector so a mark added
from then on gets a connector of its own. Nothing during the destruction
needs the connector. The inode reference of the connector is dropped
after the marks are gone as before.

Fixes: 6b3f05d24d35 ("fsnotify: Detach mark from object list when last reference is dropped")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/notify/mark.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index b2640d836a71..f6891d39e42f 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -1112,6 +1112,16 @@ void fsnotify_destroy_marks(fsnotify_connp_t *connp)
 	conn = fsnotify_grab_connector(connp);
 	if (!conn)
 		return;
+	/*
+	 * Detach the connector from the object first. Once conn->lock is
+	 * dropped a mark could be added in front of the one we're at and the
+	 * walk would miss it. fsnotify_grab_connector() refuses a detached
+	 * connector so any mark added from now on gets a connector of its
+	 * own. This also stops pinning the inode until all mark references
+	 * get dropped. It would lead to strange results such as delaying
+	 * inode deletion or blocking unmount.
+	 */
+	objp = fsnotify_detach_connector_from_object(conn, &type);
 	/*
 	 * We have to be careful since we can race with e.g.
 	 * fsnotify_clear_marks_by_group() and once we drop the conn->lock, the
@@ -1128,12 +1138,6 @@ void fsnotify_destroy_marks(fsnotify_connp_t *connp)
 		fsnotify_destroy_mark(mark, mark->group);
 		spin_lock(&conn->lock);
 	}
-	/*
-	 * Detach list from object now so that we don't pin inode until all
-	 * mark references get dropped. It would lead to strange results such
-	 * as delaying inode deletion or blocking unmount.
-	 */
-	objp = fsnotify_detach_connector_from_object(conn, &type);
 	spin_unlock(&conn->lock);
 	if (old_mark)
 		fsnotify_put_mark(old_mark);

-- 
2.53.0