nullfs: Clear inotify flags during reclaim

The inotify flags are copied from the lower vnode into the nullfs vnode
so that the INOTIFY() macro will invoke VOP_INOTIFY on the nullfs vnode;
this is then bypassed to the lower vnode.  However, when a nullfs vnode
is reclaimed we should clear these flags, as the vnode is now doomed and
no longer forwards VOPs to the lower vnode.

Add regression tests.  Remove a test in vn_inotify_revoke() which is no
longer needed after this change.

PR:		292495
Reviewed by:	kib
Reported by:	Jed Laundry <jlaundry@jlaundry.com>
Fixes:		f1f230439f ("vfs: Initial revision of inotify")
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D56639

sys/fs/nullfs/null_vnops.c

@@ -968,6 +968,7 @@ null_reclaim(struct vop_reclaim_args *ap)
 	struct vnode *vp;
 	struct null_node *xp;
 	struct vnode *lowervp;
+	short flags;
 
 	vp = ap->a_vp;
 	xp = VTONULL(vp);
@@ -997,6 +998,17 @@ null_reclaim(struct vop_reclaim_args *ap)
 	else if (vp->v_writecount < 0)
 		vp->v_writecount = 0;
 
+	/*
+	 * Undo the effects of null_copy_inotify(): setting VIRF_INOTIFY* causes
+	 * the VFS to invoke VOP_INOTIFY on the marked vnode, and for nullfs
+	 * vnodes this is bypassed to the lower vnode.  The inotify watch holds
+	 * a ref on the lower vnode, but not the upper vnode, so VOP_INOTIFY
+	 * must not be called on the upper vnode after this point.
+	 */
+	flags = vn_irflag_read(vp) & (VIRF_INOTIFY | VIRF_INOTIFY_PARENT);
+	if (flags != 0)
+		vn_irflag_unset_locked(vp, flags);
+
 	VI_UNLOCK(vp);
 
 	if ((xp->null_flags & NULLV_NOUNLOCK) != 0)

sys/kern/vfs_inotify.c

@@ -889,10 +889,6 @@ vn_inotify_add_watch(struct vnode *vp, struct inotify_softc *sc, uint32_t mask,
 void
 vn_inotify_revoke(struct vnode *vp)
 {
-	if (vp->v_pollinfo == NULL) {
-		/* This is a nullfs vnode which shadows a watched vnode. */
-		return;
-	}
 	inotify_log(vp, NULL, 0, IN_UNMOUNT, 0);
 }
 

tests/sys/kern/inotify_test.c

@@ -392,6 +392,116 @@ ATF_TC_CLEANUP(inotify_nullfs, tc)
 	}
 }
 
+/*
+ * Watch a file in a nullfs mount, and remove it from the lower mount.  Make
+ * sure that we get an IN_DELETE_SELF event and that the watch is removed.
+ */
+ATF_TC_WITH_CLEANUP(inotify_nullfs_remove);
+ATF_TC_HEAD(inotify_nullfs_remove, tc)
+{
+	atf_tc_set_md_var(tc, "require.user", "root");
+}
+ATF_TC_BODY(inotify_nullfs_remove, tc)
+{
+	char dir[PATH_MAX], path[PATH_MAX], *p;
+	int error, fd, ifd, wd;
+
+	strlcpy(dir, "./test.XXXXXX", sizeof(dir));
+	p = mkdtemp(dir);
+	ATF_REQUIRE(p == dir);
+
+	error = mkdir("./mnt", 0755);
+	ATF_REQUIRE(error == 0);
+
+	/* Mount the testdir onto ./mnt. */
+	mount_nullfs("./mnt", dir);
+
+	snprintf(path, sizeof(path), "%s/file", dir);
+	fd = open(path, O_RDWR | O_CREAT, 0644);
+	ATF_REQUIRE(fd != -1);
+	close_checked(fd);
+
+	ifd = inotify(IN_NONBLOCK);
+	wd = inotify_add_watch(ifd, "./mnt/file", IN_DELETE_SELF);
+	ATF_REQUIRE(wd != -1);
+
+	error = unlink(path);
+	ATF_REQUIRE(error == 0);
+
+	consume_event(ifd, wd, IN_DELETE_SELF, 0, NULL);
+	consume_event(ifd, wd, 0, IN_IGNORED, NULL);
+
+	close_inotify(ifd);
+}
+ATF_TC_CLEANUP(inotify_nullfs_remove, tc)
+{
+	int error;
+
+	error = unmount("./mnt", 0);
+	if (error != 0) {
+		perror("unmount");
+		exit(1);
+	}
+}
+
+/*
+ * Exercise a scenario where a watched lower vnode is deleted by a rename.  The
+ * deletion causes the upper vnode to be reclaimed, and after that point it
+ * should stop trying to forward events back to the (now detached) lower vnode.
+ */
+ATF_TC_WITH_CLEANUP(inotify_nullfs_rename);
+ATF_TC_HEAD(inotify_nullfs_rename, tc)
+{
+	atf_tc_set_md_var(tc, "require.user", "root");
+}
+ATF_TC_BODY(inotify_nullfs_rename, tc)
+{
+	char dir[PATH_MAX], path1[PATH_MAX], path2[PATH_MAX], *p;
+	int error, fd, ifd, wd;
+
+	strlcpy(dir, "./test.XXXXXX", sizeof(dir));
+	p = mkdtemp(dir);
+	ATF_REQUIRE(p == dir);
+
+	error = mkdir("./mnt", 0755);
+	ATF_REQUIRE(error == 0);
+
+	/* Mount the testdir onto ./mnt. */
+	mount_nullfs("./mnt", dir);
+
+	ifd = inotify(IN_NONBLOCK);
+
+	/* Create two files, they will be renamed in the upper layer. */
+	snprintf(path1, sizeof(path1), "%s/file1", dir);
+	fd = open(path1, O_RDWR | O_CREAT, 0644);
+	ATF_REQUIRE(fd != -1);
+	close_checked(fd);
+	snprintf(path2, sizeof(path2), "%s/file2", dir);
+	fd = open(path2, O_RDWR | O_CREAT, 0644);
+	ATF_REQUIRE(fd != -1);
+	close_checked(fd);
+
+	wd = inotify_add_watch(ifd, "./mnt/file1", IN_DELETE_SELF);
+	ATF_REQUIRE(wd != -1);
+	error = rename("./mnt/file2", "./mnt/file1");
+	ATF_REQUIRE(error == 0);
+
+	consume_event(ifd, wd, IN_DELETE_SELF, 0, NULL);
+	consume_event(ifd, wd, 0, IN_IGNORED, NULL);
+
+	close_inotify(ifd);
+}
+ATF_TC_CLEANUP(inotify_nullfs_rename, tc)
+{
+	int error;
+
+	error = unmount("./mnt", 0);
+	if (error != 0) {
+		perror("unmount");
+		exit(1);
+	}
+}
+
 /*
  * Make sure that exceeding max_events pending events results in an overflow
  * event.
@@ -878,6 +988,8 @@ ATF_TP_ADD_TCS(tp)
 	ATF_TP_ADD_TC(tp, inotify_coalesce);
 	ATF_TP_ADD_TC(tp, inotify_mask_create);
 	ATF_TP_ADD_TC(tp, inotify_nullfs);
+	ATF_TP_ADD_TC(tp, inotify_nullfs_remove);
+	ATF_TP_ADD_TC(tp, inotify_nullfs_rename);
 	ATF_TP_ADD_TC(tp, inotify_queue_overflow);
 	/* Tests for the various inotify event types. */
 	ATF_TP_ADD_TC(tp, inotify_event_access_file);