Skip to contentSkip to conversation

FAILED: patch "[PATCH] namespace: unhash a dentry before detaching the mounts on it" failed to apply to 5.10-stable tree

PATCH New [email protected] 2 participants 2 messages Oct 12 – Oct 14

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 528b383dc35d1e5caf64ddf5d35f983ae543b0d0
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026101212-snap-scoop-0999@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 528b383dc35d1e5caf64ddf5d35f983ae543b0d0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <[email protected]>
Date: Fri, 2 Oct 2026 15:52:32 +0200
Subject: [PATCH] namespace: unhash a dentry before detaching the mounts on it

unlink(), rmdir() and rename() remove the entry from the filesystem, call detach_mounts() on the entry with the inode locked and then call d_delete() once the inode is unlocked.

The thing is that between detach_mounts() and d_delete() the dentry is still hashed and positive. But after detach_mounts() nothing covers the dentry anymore. A lookup that finds this dentry in the dcache can uncover what the mounts hid.

That's a problem when unlinking files or directories that are mountpoints in other mount namespaces. Everybody who had the underlying entry covered can race the detach_mount() call until d_delete() has run.

The race window isn't all that small. It encompasses namespace_unlock() with a full synchronize_rcu_expedited() grace period and the inode_unlock() of the entry.

In my experiments three walkers that kept trying read an overmounted file in 11938 of the 16140 unlinks that removed its mountpoint from a bind mount of the filesystem within a minute.

Fun fact, d_invalidate() has the same ordering problem but gets it right. It unhashes the dentry first and detaches the mounts afterwards. Let's do the same in __detach_mounts():

- A lookup that hasn't found the dentry yet misses it in the dcache and waits for the directory lock that the caller holds until the name is gone for good.

- A lockless lookup that found it already fails the mount_lock check while the dentry still counts as a mountpoint and the d_seq check once it doesn't, and retries.

This fixes the lockless path. We still need to fix the reference count lookup in a follow-up patch.

So d_drop() the dentry. The dentry stays positive and held but can't be found anymore. Then proceed with the detach and unlink.

Reproducer:

The reads were counted with the program below, three walkers for 60 s in a VM with 4 CPUs. The race window is stretched with the debug patch pasted here. fs.detach_race_walk_us sleeps in lookup_fast() once it found a mountpoint dentry and before its mounts are crossed. fs.detach_race_unlink_us sleeps in vfs_unlink() between detach_mounts() and d_delete().

  // SPDX-License-Identifier: GPL-2.0
  /*
   * unlink_covered: unlink a file that is a mountpoint in a detached copy of
   * its mount, against lookups that open it through that copy.
   *
   * A is a tmpfs with the file f1 (content SECRET_F) and the plain file
   * MARK_A. A' is an open_tree(OPEN_TREE_CLONE) copy of A with MARK_A bound
   * on A'/f1, held through an O_PATH fd on its root once the tree fd is
   * closed. Walkers open f1 through that fd while the driver unlinks A/f1,
   * where nothing is mounted on it. A walker may read MARK_A or get ENOENT.
   * A read of SECRET_F is a hit: the name was found after its mount was
   * gone. The lockless walks use openat2(RESOLVE_CACHED).
   *
   * usage: unlink_covered [-t seconds] [-w walkers]
   */
  #ifndef _GNU_SOURCE
  #define _GNU_SOURCE
  #endif
  #include <errno.h>
  #include <fcntl.h>
  #include <pthread.h>
  #include <stdatomic.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
  #include <unistd.h>
  #include <sys/mount.h>
  #include <sys/stat.h>
  #include <sys/syscall.h>
  #include <linux/openat2.h>

  #ifndef __NR_open_tree
  #define __NR_open_tree 428
  #endif
  #ifndef __NR_move_mount
  #define __NR_move_mount 429
  #endif
  #ifndef __NR_openat2
  #define __NR_openat2 437
  #endif
  #ifndef OPEN_TREE_CLONE
  #define OPEN_TREE_CLONE 1
  #endif
  #ifndef OPEN_TREE_CLOEXEC
  #define OPEN_TREE_CLOEXEC O_CLOEXEC
  #endif
  #ifndef MOVE_MOUNT_F_EMPTY_PATH
  #define MOVE_MOUNT_F_EMPTY_PATH 0x00000004
  #endif

  #define WORK "/tmp/uc"
  #define ADIR WORK "/A"

static int duration = 60, nwalkers = 3;
static atomic_int stop, writer_waiting;
static pthread_rwlock_t cur_lock = PTHREAD_RWLOCK_INITIALIZER;
static int cur_fd = -1; /* the root of A', -1 while there is none */
static atomic_long n_unlink, n_walk, n_mark, n_enoent, n_other, n_secret,
n_secret_cached;

  static void die(const char *what)
  {
  	fprintf(stderr, "FATAL %s: %s\n", what, strerror(errno));
  	exit(2);
  }

static void put_file(int dfd, const char *name, const char *content) {
int fd = openat(dfd, name, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
0644);

  	if (fd < 0 || write(fd, content, strlen(content)) < 0)
  		die(name);
  	close(fd);
  }

  /* "plain/../" @n times, then f1: a longer walk that checks nothing on its way */
  static char *longpath(int n)
  {
  	char *p = malloc(n * 9 + 3), *q = p;

  	for (int i = 0; i < n; i++, q += 9)
  		memcpy(q, "plain/../", 9);
  	strcpy(q, "f1");
  	return p;
  }

  static void try_read(int dfd, const char *path, int cached)
  {
  	struct open_how how = { .flags = O_RDONLY | O_CLOEXEC,
  				.resolve = RESOLVE_CACHED };
  	char buf[32] = "";
  	long n;
  	int fd;

  	if (cached)
  		fd = syscall(__NR_openat2, dfd, path, &how, sizeof(how));
  	else
  		fd = openat(dfd, path, O_RDONLY | O_CLOEXEC);
  	atomic_fetch_add(&n_walk, 1);
  	if (fd < 0) {
  		if (errno == ENOENT)
  			atomic_fetch_add(&n_enoent, 1);
  		else if (!cached || errno != EAGAIN)
  			atomic_fetch_add(&n_other, 1);
  		return;
  	}
  	n = read(fd, buf, sizeof(buf) - 1);
  	close(fd);
  	if (n >= 6 && !strncmp(buf, "SECRET", 6)) {
  		if (cached)
  			atomic_fetch_add(&n_secret_cached, 1);
  		if (!atomic_fetch_add(&n_secret, 1))
  			printf("HIT: read \"%s\" through %s (%s walk)\n", buf,
  			       path, cached ? "lockless" : "any");
  	} else {
  		atomic_fetch_add(&n_mark, 1);
  	}
  }

  static void *walker(void *arg)
  {
  	unsigned int r = (long)arg * 2654435761u;

  	while (!atomic_load(&stop)) {
  		char *p_long, *p_short;
  		int dfd;

  		while (atomic_load(&writer_waiting) && !atomic_load(&stop))
  			usleep(20);
  		pthread_rwlock_rdlock(&cur_lock);
  		dfd = cur_fd;
  		if (dfd < 0) {
  			pthread_rwlock_unlock(&cur_lock);
  			usleep(100);
  			continue;
  		}
  		r = r * 1103515245u + 12345u;
  		p_long = longpath(1 + (r >> 8) % 400);
  		p_short = longpath(0);
  		try_read(dfd, p_long, 0);
  		try_read(dfd, p_short, 0);
  		try_read(dfd, p_long, 1);
  		pthread_rwlock_unlock(&cur_lock);
  		free(p_long);
  		free(p_short);
  	}
  	return NULL;
  }

/* hand the walkers a new A' (or none), close the old one */ static void publish(int fd)
{
int old;

  	atomic_fetch_add(&writer_waiting, 1);
  	pthread_rwlock_wrlock(&cur_lock);
  	old = cur_fd;
  	cur_fd = fd;
  	pthread_rwlock_unlock(&cur_lock);
  	atomic_fetch_sub(&writer_waiting, 1);
  	if (old >= 0)
  		close(old);
  }

  static void *driver(void *arg __attribute__((unused)))
  {
  	int a;

  	if (mkdir(ADIR, 0755) && errno != EEXIST)
  		die("mkdir A");
  	if (mount("A", ADIR, "tmpfs", 0, "size=4M"))
  		die("mount A");
  	a = open(ADIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
  	if (a < 0 || mkdirat(a, "plain", 0755))
  		die("A/plain");
  	put_file(a, "MARK_A", "MARK_A");
  	while (!atomic_load(&stop)) {
  		int t, m, fd_a;

  		put_file(a, "f1", "SECRET_F");
  		t = syscall(__NR_open_tree, a, "",
  			    OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC | AT_EMPTY_PATH);
  		if (t < 0)
  			die("open_tree A");
  		m = syscall(__NR_open_tree, a, "MARK_A",
  			    OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
  		if (m < 0)
  			die("open_tree MARK_A");
  		if (syscall(__NR_move_mount, m, "", t, "f1",
  			    MOVE_MOUNT_F_EMPTY_PATH))
  			die("move_mount");
  		close(m);
  		fd_a = openat(t, ".", O_PATH | O_DIRECTORY | O_CLOEXEC);
  		if (fd_a < 0)
  			die("open A'");
  		publish(fd_a);
  		usleep(200 + rand() % 1000);
  		close(t);			/* A' is unmounted, fd_a holds it */
  		usleep(200 + rand() % 1000);
  		if (unlinkat(a, "f1", 0))	/* through A, a plain file there */
  			die("unlink f1");
  		atomic_fetch_add(&n_unlink, 1);
  		usleep(rand() % 300);
  		publish(-1);
  	}
  	close(a);
  	umount2(ADIR, MNT_DETACH);
  	return NULL;
  }

  int main(int argc, char **argv)
  {
  	pthread_t d, *w;
  	int c, i;

  	setvbuf(stdout, NULL, _IOLBF, 0);
  	while ((c = getopt(argc, argv, "t:w:")) != -1) {
  		switch (c) {
  		case 't':
  			duration = atoi(optarg);
  			break;
  		case 'w':
  			nwalkers = atoi(optarg);
  			break;
  		default:
  			fprintf(stderr, "usage: unlink_covered [-t seconds] [-w walkers]\n");
  			return 2;
  		}
  	}
  	if (unshare(CLONE_NEWNS) ||
  	    mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
  		die("unshare");
  	if (mkdir(WORK, 0755) && errno != EEXIST)
  		die("mkdir");
  	w = calloc(nwalkers, sizeof(*w));
  	for (i = 0; i < nwalkers; i++)
  		if (pthread_create(&w[i], NULL, walker, (void *)(long)i))
  			die("pthread_create");
  	if (pthread_create(&d, NULL, driver, NULL))
  		die("pthread_create");
  	sleep(duration);
  	atomic_store(&stop, 1);
  	pthread_join(d, NULL);
  	for (i = 0; i < nwalkers; i++)
  		pthread_join(w[i], NULL);
  	printf("unlink_covered: %d s, %d walkers: unlinks %ld walks %ld mark %ld enoent %ld other %ld SECRET %ld (lockless %ld)\n",
  	       duration, nwalkers, atomic_load(&n_unlink), atomic_load(&n_walk),
  	       atomic_load(&n_mark), atomic_load(&n_enoent),
  	       atomic_load(&n_other), atomic_load(&n_secret),
  	       atomic_load(&n_secret_cached));
  	return atomic_load(&n_secret) ? 1 : 0;
  }

  diff --git a/fs/namei.c b/fs/namei.c
  --- a/fs/namei.c
  +++ b/fs/namei.c
  @@ -35,6 +35,7 @@
   #include <linux/fcntl.h>
   #include <linux/device_cgroup.h>
   #include <linux/fs_struct.h>
  +#include <linux/delay.h>
   #include <linux/posix_acl.h>
   #include <linux/hash.h>
   #include <linux/bitops.h>
  @@ -1205,9 +1206,26 @@ static int sysctl_protected_symlinks __read_mostly;
   static int sysctl_protected_hardlinks __read_mostly;
   static int sysctl_protected_fifos __read_mostly;
   static int sysctl_protected_regular __read_mostly;
  +/* debug: widen the two windows of the detach_mounts() race */
  +static int sysctl_detach_race_walk_us __read_mostly;
  +static int sysctl_detach_race_unlink_us __read_mostly;
   #ifdef CONFIG_SYSCTL
   static const struct ctl_table namei_sysctls[] = {
  +	{
  +		.procname	= "detach_race_walk_us",
  +		.data		= &sysctl_detach_race_walk_us,
  +		.maxlen		= sizeof(int),
  +		.mode		= 0644,
  +		.proc_handler	= proc_dointvec,
  +	},
  +	{
  +		.procname	= "detach_race_unlink_us",
  +		.data		= &sysctl_detach_race_unlink_us,
  +		.maxlen		= sizeof(int),
  +		.mode		= 0644,
  +		.proc_handler	= proc_dointvec,
  +	},
   	{
   		.procname	= "protected_symlinks",
   		.data		= &sysctl_protected_symlinks,
  @@ -1878,6 +1896,10 @@ static struct dentry *lookup_fast(struct nameidata *nd)
   		dentry = __d_lookup(parent, &nd->last);
   		if (unlikely(!dentry))
   			return NULL;
  +		/* debug: between finding a mountpoint and crossing its mounts */
  +		if (unlikely(sysctl_detach_race_walk_us) && d_mountpoint(dentry))
  +			usleep_range(sysctl_detach_race_walk_us,
  +				     sysctl_detach_race_walk_us + 10);
   		status = d_revalidate(nd->inode, &nd->last, dentry, nd->flags);
   	}
   	if (unlikely(status <= 0)) {
  @@ -5693,6 +5715,10 @@ int vfs_unlink(struct mnt_idmap *idmap, struct inode *dir,
   			if (!error) {
   				dont_mount(dentry);
   				detach_mounts(dentry);
  +				/* debug: the mounts are gone, d_delete() is still to come */
  +				if (unlikely(sysctl_detach_race_unlink_us))
  +					usleep_range(sysctl_detach_race_unlink_us,
  +						     sysctl_detach_race_unlink_us + 10);
   			}
   		}
   	}
Fixes 8ed936b5671b ("vfs: Lazily remove mounts on unlinked files and directories.")
fs/namespace.c +5 -0
index fcf42f192aae..e576a5d6eff0 100644
@@ -1996,6 +1996,9 @@ static int do_umount(struct mount *mnt, int flags)
  * detach_mounts allows lazily unmounting those mounts instead of
  * leaking them.
  *
+ * The dentry is unhashed before the mounts go so that no lookup finds
+ * what they covered. The caller removes it for good afterwards.
+ *
  * The caller may hold dentry->d_inode->i_rwsem.
  */
 void __detach_mounts(struct dentry *dentry)
@@ -2009,6 +2012,8 @@ void __detach_mounts(struct dentry *dentry)
 	if (!lookup_mountpoint(dentry, &mp))
 		return;
 
+	/* the name goes first, what covered it goes second */
+	d_drop(dentry);
 	event++;
 	while (mp.node.next) {
 		mnt = hlist_entry(mp.node.next, struct mount, mnt_mp_list);
maintainer
#1 lore raw

PATCH 1/1 namespace: unhash a dentry before detaching the mounts on it

From: Christian Brauner <[email protected]>

[ Upstream commit 528b383dc35d1e5caf64ddf5d35f983ae543b0d0 ]

unlink(), rmdir() and rename() remove the entry from the filesystem, call detach_mounts() on the entry with the inode locked and then call d_delete() once the inode is unlocked.

The thing is that between detach_mounts() and d_delete() the dentry is still hashed and positive. But after detach_mounts() nothing covers the dentry anymore. A lookup that finds this dentry in the dcache can uncover what the mounts hid.

That's a problem when unlinking files or directories that are mountpoints in other mount namespaces. Everybody who had the underlying entry covered can race the detach_mount() call until d_delete() has run.

The race window isn't all that small. It encompasses namespace_unlock() with a full synchronize_rcu_expedited() grace period and the inode_unlock() of the entry.

In my experiments three walkers that kept trying read an overmounted file in 11938 of the 16140 unlinks that removed its mountpoint from a bind mount of the filesystem within a minute.

Fun fact, d_invalidate() has the same ordering problem but gets it right. It unhashes the dentry first and detaches the mounts afterwards. Let's do the same in __detach_mounts():

- A lookup that hasn't found the dentry yet misses it in the dcache and waits for the directory lock that the caller holds until the name is gone for good.

- A lockless lookup that found it already fails the mount_lock check while the dentry still counts as a mountpoint and the d_seq check once it doesn't, and retries.

This fixes the lockless path. We still need to fix the reference count lookup in a follow-up patch.

So d_drop() the dentry. The dentry stays positive and held but can't be found anymore. Then proceed with the detach and unlink.

Reproducer:

The reads were counted with the program below, three walkers for 60 s in a VM with 4 CPUs. The race window is stretched with the debug patch pasted here. fs.detach_race_walk_us sleeps in lookup_fast() once it found a mountpoint dentry and before its mounts are crossed. fs.detach_race_unlink_us sleeps in vfs_unlink() between detach_mounts() and d_delete().

  // SPDX-License-Identifier: GPL-2.0
  /*
   * unlink_covered: unlink a file that is a mountpoint in a detached copy of
   * its mount, against lookups that open it through that copy.
   *
   * A is a tmpfs with the file f1 (content SECRET_F) and the plain file
   * MARK_A. A' is an open_tree(OPEN_TREE_CLONE) copy of A with MARK_A bound
   * on A'/f1, held through an O_PATH fd on its root once the tree fd is
   * closed. Walkers open f1 through that fd while the driver unlinks A/f1,
   * where nothing is mounted on it. A walker may read MARK_A or get ENOENT.
   * A read of SECRET_F is a hit: the name was found after its mount was
   * gone. The lockless walks use openat2(RESOLVE_CACHED).
   *
   * usage: unlink_covered [-t seconds] [-w walkers]
   */
  #ifndef _GNU_SOURCE
  #define _GNU_SOURCE
  #endif
  #include <errno.h>
  #include <fcntl.h>
  #include <pthread.h>
  #include <stdatomic.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
  #include <unistd.h>
  #include <sys/mount.h>
  #include <sys/stat.h>
  #include <sys/syscall.h>
  #include <linux/openat2.h>

  #ifndef __NR_open_tree
  #define __NR_open_tree 428
  #endif
  #ifndef __NR_move_mount
  #define __NR_move_mount 429
  #endif
  #ifndef __NR_openat2
  #define __NR_openat2 437
  #endif
  #ifndef OPEN_TREE_CLONE
  #define OPEN_TREE_CLONE 1
  #endif
  #ifndef OPEN_TREE_CLOEXEC
  #define OPEN_TREE_CLOEXEC O_CLOEXEC
  #endif
  #ifndef MOVE_MOUNT_F_EMPTY_PATH
  #define MOVE_MOUNT_F_EMPTY_PATH 0x00000004
  #endif

  #define WORK "/tmp/uc"
  #define ADIR WORK "/A"

static int duration = 60, nwalkers = 3;
static atomic_int stop, writer_waiting;
static pthread_rwlock_t cur_lock = PTHREAD_RWLOCK_INITIALIZER;
static int cur_fd = -1; /* the root of A', -1 while there is none */
static atomic_long n_unlink, n_walk, n_mark, n_enoent, n_other, n_secret,
n_secret_cached;

  static void die(const char *what)
  {
  	fprintf(stderr, "FATAL %s: %s\n", what, strerror(errno));
  	exit(2);
  }

static void put_file(int dfd, const char *name, const char *content) {
int fd = openat(dfd, name, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
0644);

  	if (fd < 0 || write(fd, content, strlen(content)) < 0)
  		die(name);
  	close(fd);
  }

  /* "plain/../" @n times, then f1: a longer walk that checks nothing on its way */
  static char *longpath(int n)
  {
  	char *p = malloc(n * 9 + 3), *q = p;

  	for (int i = 0; i < n; i++, q += 9)
  		memcpy(q, "plain/../", 9);
  	strcpy(q, "f1");
  	return p;
  }

  static void try_read(int dfd, const char *path, int cached)
  {
  	struct open_how how = { .flags = O_RDONLY | O_CLOEXEC,
  				.resolve = RESOLVE_CACHED };
  	char buf[32] = "";
  	long n;
  	int fd;

  	if (cached)
  		fd = syscall(__NR_openat2, dfd, path, &how, sizeof(how));
  	else
  		fd = openat(dfd, path, O_RDONLY | O_CLOEXEC);
  	atomic_fetch_add(&n_walk, 1);
  	if (fd < 0) {
  		if (errno == ENOENT)
  			atomic_fetch_add(&n_enoent, 1);
  		else if (!cached || errno != EAGAIN)
  			atomic_fetch_add(&n_other, 1);
  		return;
  	}
  	n = read(fd, buf, sizeof(buf) - 1);
  	close(fd);
  	if (n >= 6 && !strncmp(buf, "SECRET", 6)) {
  		if (cached)
  			atomic_fetch_add(&n_secret_cached, 1);
  		if (!atomic_fetch_add(&n_secret, 1))
  			printf("HIT: read \"%s\" through %s (%s walk)\n", buf,
  			       path, cached ? "lockless" : "any");
  	} else {
  		atomic_fetch_add(&n_mark, 1);
  	}
  }

  static void *walker(void *arg)
  {
  	unsigned int r = (long)arg * 2654435761u;

  	while (!atomic_load(&stop)) {
  		char *p_long, *p_short;
  		int dfd;

  		while (atomic_load(&writer_waiting) && !atomic_load(&stop))
  			usleep(20);
  		pthread_rwlock_rdlock(&cur_lock);
  		dfd = cur_fd;
  		if (dfd < 0) {
  			pthread_rwlock_unlock(&cur_lock);
  			usleep(100);
  			continue;
  		}
  		r = r * 1103515245u + 12345u;
  		p_long = longpath(1 + (r >> 8) % 400);
  		p_short = longpath(0);
  		try_read(dfd, p_long, 0);
  		try_read(dfd, p_short, 0);
  		try_read(dfd, p_long, 1);
  		pthread_rwlock_unlock(&cur_lock);
  		free(p_long);
  		free(p_short);
  	}
  	return NULL;
  }

/* hand the walkers a new A' (or none), close the old one */ static void publish(int fd)
{
int old;

  	atomic_fetch_add(&writer_waiting, 1);
  	pthread_rwlock_wrlock(&cur_lock);
  	old = cur_fd;
  	cur_fd = fd;
  	pthread_rwlock_unlock(&cur_lock);
  	atomic_fetch_sub(&writer_waiting, 1);
  	if (old >= 0)
  		close(old);
  }

  static void *driver(void *arg __attribute__((unused)))
  {
  	int a;

  	if (mkdir(ADIR, 0755) && errno != EEXIST)
  		die("mkdir A");
  	if (mount("A", ADIR, "tmpfs", 0, "size=4M"))
  		die("mount A");
  	a = open(ADIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
  	if (a < 0 || mkdirat(a, "plain", 0755))
  		die("A/plain");
  	put_file(a, "MARK_A", "MARK_A");
  	while (!atomic_load(&stop)) {
  		int t, m, fd_a;

  		put_file(a, "f1", "SECRET_F");
  		t = syscall(__NR_open_tree, a, "",
  			    OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC | AT_EMPTY_PATH);
  		if (t < 0)
  			die("open_tree A");
  		m = syscall(__NR_open_tree, a, "MARK_A",
  			    OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
  		if (m < 0)
  			die("open_tree MARK_A");
  		if (syscall(__NR_move_mount, m, "", t, "f1",
  			    MOVE_MOUNT_F_EMPTY_PATH))
  			die("move_mount");
  		close(m);
  		fd_a = openat(t, ".", O_PATH | O_DIRECTORY | O_CLOEXEC);
  		if (fd_a < 0)
  			die("open A'");
  		publish(fd_a);
  		usleep(200 + rand() % 1000);
  		close(t);			/* A' is unmounted, fd_a holds it */
  		usleep(200 + rand() % 1000);
  		if (unlinkat(a, "f1", 0))	/* through A, a plain file there */
  			die("unlink f1");
  		atomic_fetch_add(&n_unlink, 1);
  		usleep(rand() % 300);
  		publish(-1);
  	}
  	close(a);
  	umount2(ADIR, MNT_DETACH);
  	return NULL;
  }

  int main(int argc, char **argv)
  {
  	pthread_t d, *w;
  	int c, i;

  	setvbuf(stdout, NULL, _IOLBF, 0);
  	while ((c = getopt(argc, argv, "t:w:")) != -1) {
  		switch (c) {
  		case 't':
  			duration = atoi(optarg);
  			break;
  		case 'w':
  			nwalkers = atoi(optarg);
  			break;
  		default:
  			fprintf(stderr, "usage: unlink_covered [-t seconds] [-w walkers]\n");
  			return 2;
  		}
  	}
  	if (unshare(CLONE_NEWNS) ||
  	    mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
  		die("unshare");
  	if (mkdir(WORK, 0755) && errno != EEXIST)
  		die("mkdir");
  	w = calloc(nwalkers, sizeof(*w));
  	for (i = 0; i < nwalkers; i++)
  		if (pthread_create(&w[i], NULL, walker, (void *)(long)i))
  			die("pthread_create");
  	if (pthread_create(&d, NULL, driver, NULL))
  		die("pthread_create");
  	sleep(duration);
  	atomic_store(&stop, 1);
  	pthread_join(d, NULL);
  	for (i = 0; i < nwalkers; i++)
  		pthread_join(w[i], NULL);
  	printf("unlink_covered: %d s, %d walkers: unlinks %ld walks %ld mark %ld enoent %ld other %ld SECRET %ld (lockless %ld)\n",
  	       duration, nwalkers, atomic_load(&n_unlink), atomic_load(&n_walk),
  	       atomic_load(&n_mark), atomic_load(&n_enoent),
  	       atomic_load(&n_other), atomic_load(&n_secret),
  	       atomic_load(&n_secret_cached));
  	return atomic_load(&n_secret) ? 1 : 0;
  }

  diff --git a/fs/namei.c b/fs/namei.c
  --- a/fs/namei.c
  +++ b/fs/namei.c
  @@ -35,6 +35,7 @@
   #include <linux/fcntl.h>
   #include <linux/device_cgroup.h>
   #include <linux/fs_struct.h>
  +#include <linux/delay.h>
   #include <linux/posix_acl.h>
   #include <linux/hash.h>
   #include <linux/bitops.h>
  @@ -1205,9 +1206,26 @@ static int sysctl_protected_symlinks __read_mostly;
   static int sysctl_protected_hardlinks __read_mostly;
   static int sysctl_protected_fifos __read_mostly;
   static int sysctl_protected_regular __read_mostly;
  +/* debug: widen the two windows of the detach_mounts() race */
  +static int sysctl_detach_race_walk_us __read_mostly;
  +static int sysctl_detach_race_unlink_us __read_mostly;
   #ifdef CONFIG_SYSCTL
   static const struct ctl_table namei_sysctls[] = {
  +	{
  +		.procname	= "detach_race_walk_us",
  +		.data		= &sysctl_detach_race_walk_us,
  +		.maxlen		= sizeof(int),
  +		.mode		= 0644,
  +		.proc_handler	= proc_dointvec,
  +	},
  +	{
  +		.procname	= "detach_race_unlink_us",
  +		.data		= &sysctl_detach_race_unlink_us,
  +		.maxlen		= sizeof(int),
  +		.mode		= 0644,
  +		.proc_handler	= proc_dointvec,
  +	},
   	{
   		.procname	= "protected_symlinks",
   		.data		= &sysctl_protected_symlinks,
  @@ -1878,6 +1896,10 @@ static struct dentry *lookup_fast(struct nameidata *nd)
   		dentry = __d_lookup(parent, &nd->last);
   		if (unlikely(!dentry))
   			return NULL;
  +		/* debug: between finding a mountpoint and crossing its mounts */
  +		if (unlikely(sysctl_detach_race_walk_us) && d_mountpoint(dentry))
  +			usleep_range(sysctl_detach_race_walk_us,
  +				     sysctl_detach_race_walk_us + 10);
   		status = d_revalidate(nd->inode, &nd->last, dentry, nd->flags);
   	}
   	if (unlikely(status <= 0)) {
  @@ -5693,6 +5715,10 @@ int vfs_unlink(struct mnt_idmap *idmap, struct inode *dir,
   			if (!error) {
   				dont_mount(dentry);
   				detach_mounts(dentry);
  +				/* debug: the mounts are gone, d_delete() is still to come */
  +				if (unlikely(sysctl_detach_race_unlink_us))
  +					usleep_range(sysctl_detach_race_unlink_us,
  +						     sysctl_detach_race_unlink_us + 10);
   			}
   		}
   	}
Fixes 8ed936b5671b ("vfs: Lazily remove mounts on unlinked files and directories.")
[ sashal: adjusted context for pointer-based mountpoint handling ]
---
 fs/namespace.c | 5 +++++
 1 file changed, 5 insertions(+)
fs/namespace.c +5 -0
index 769f22c0bc66d..42fb75ec6b91f 100644
@@ -1666,6 +1666,9 @@ static int do_umount(struct mount *mnt, int flags)
  * detach_mounts allows lazily unmounting those mounts instead of
  * leaking them.
  *
+ * The dentry is unhashed before the mounts go so that no lookup finds
+ * what they covered. The caller removes it for good afterwards.
+ *
  * The caller may hold dentry->d_inode->i_mutex.
  */
 void __detach_mounts(struct dentry *dentry)
@@ -1679,6 +1682,8 @@ void __detach_mounts(struct dentry *dentry)
 	if (!mp)
 		goto out_unlock;
 
+	/* the name goes first, what covered it goes second */
+	d_drop(dentry);
 	event++;
 	while (!hlist_empty(&mp->m_list)) {
 		mnt = hlist_entry(mp->m_list.first, struct mount, mnt_mp_list);
Signature · 1 line
2.53.0