Since commit 8189ab688f5a ("namei: don't reveal overmounted entries in refwalk") a refwalk refuses any dentry that was unlinked or renamed over after it found it and the walk is repeated with LOOKUP_REVAL. But dont_mount() is called for every name that goes away, not just for mountpoints. So any unlink or rename that races with a refwalk sends it through another full walk with LOOKUP_REVAL. On NFS or FUSE that revalidates every component of the path again. Only a dentry that still had mounts when its name went away can reveal what they covered. So let __detach_mounts() mark the dentry with DCACHE_UNCOVERED when it unhashes it, before the mounts are detached, and only refuse a dentry that carries the mark. The mark lives in d_flags just like DCACHE_MOUNTED and is set under d_lock before the mounts go. So a walk that doesn't find the mounts anymore sees the mark too. A dentry that had no mounts when its name went away is stepped onto like before 8189ab688f5a. The walk found the name before it was removed and there was nothing to cross. The reproducer below has twelve threads unlink a name on tmpfs and create it again while twelve more open it with O_CREAT. Counting the walks that are repeated with LOOKUP_REVAL with cd /sys/kernel/tracing echo 'p:reval path_openat flags=%dx:x32' >> kprobe_events echo 'hist:keys=flags if flags & 0x80' > events/kprobes/reval/trigger echo 1 > events/kprobes/reval/enable ./unlink_storm -t 300 cat events/kprobes/reval/hist finds 463 of them in 16.8 million opens on a KASAN kernel. With this patch there are none in 16.9 million. // SPDX-License-Identifier: GPL-2.0 /* * unlink_storm: refwalks racing unlink() and create of a single name. * * Writers unlink "N" and create it again, readers open it with O_CREAT. * None of the opens may fail. Count the walks that were repeated with * LOOKUP_REVAL with a kprobe on path_openat(). * * usage: unlink_storm [-t secs] [-w writers] [-r readers] */ #define _GNU_SOURCE #include #include #include #include #include #include #include #include #include #include #define DIR "/tmp/us" static atomic_int stop; static atomic_long opens, failed; static atomic_int first_err; static void *writer(void *arg) { while (!atomic_load(&stop)) { int fd; unlink(DIR "/N"); fd = open(DIR "/N", O_CREAT | O_RDWR | O_CLOEXEC, 0644); if (fd >= 0) close(fd); } return arg; } static void *reader(void *arg) { while (!atomic_load(&stop)) { int fd = open(DIR "/N", O_CREAT | O_RDWR | O_CLOEXEC, 0644); if (fd < 0) { int none = 0; atomic_compare_exchange_strong(&first_err, &none, errno); atomic_fetch_add(&failed, 1); } else { close(fd); } atomic_fetch_add(&opens, 1); } return arg; } int main(int argc, char **argv) { int secs = 60, nw = 12, nr = 12, c, n = 0; pthread_t t[256]; while ((c = getopt(argc, argv, "t:w:r:")) != -1) { switch (c) { case 't': secs = atoi(optarg); break; case 'w': nw = atoi(optarg); break; case 'r': nr = atoi(optarg); break; default: return 2; } } if (nw + nr > 256) return 2; mkdir(DIR, 0755); if (mount("us", DIR, "tmpfs", 0, NULL)) { perror("tmpfs"); return 2; } for (int i = 0; i < nw; i++) pthread_create(&t[n++], NULL, writer, NULL); for (int i = 0; i < nr; i++) pthread_create(&t[n++], NULL, reader, NULL); sleep(secs); atomic_store(&stop, 1); for (int i = 0; i < n; i++) pthread_join(t[i], NULL); printf("%ds: %ld opens, %ld failed%s%s\n", secs, atomic_load(&opens), atomic_load(&failed), first_err ? ", the first with " : "", first_err ? strerror(first_err) : ""); umount2(DIR, MNT_DETACH); return atomic_load(&failed) ? 1 : 0; } Fixes: 8189ab688f5a ("namei: don't reveal overmounted entries in refwalk") Signed-off-by: Christian Brauner (Amutable) --- fs/namei.c | 11 ++++++++--- fs/namespace.c | 8 ++++++-- include/linux/dcache.h | 3 ++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index c87c14ee1a25..9efc7426ebfb 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2092,6 +2092,11 @@ static noinline const char *pick_link(struct nameidata *nd, struct path *link, * don't reveal the underyling dentry during refwalk. In rcuwalk we * catch this via d_seq and another lookup for the name. Give the same * guarantee in refwalk. + * + * Only a dentry that still had mounts when its name went away matters. + * __detach_mounts() marks it before the mounts are detached, so a walk + * that doesn't find them anymore sees the mark. Any other dentry is fine + * to step onto. The walk found the name before it was removed. */ static bool unlink_may_reveal(struct nameidata *nd, int flags, struct path *path, struct dentry *dentry) @@ -2104,10 +2109,10 @@ static bool unlink_may_reveal(struct nameidata *nd, int flags, /* We crossed onto a mount and the name led us here while it still existed */ if (path->mnt != nd->path.mnt) return false; - /* only a name on its way out is flagged */ - if (likely(!cant_mount(dentry))) + /* only a name that went away while covered is flagged */ + if (likely(!(READ_ONCE(dentry->d_flags) & DCACHE_UNCOVERED))) return false; - if (!d_unlinked(dentry)) + if (!cant_mount(dentry) || !d_unlinked(dentry)) return false; dput(no_free_ptr(path->dentry)); return true; diff --git a/fs/namespace.c b/fs/namespace.c index 3ed3766f3a18..54dcdb88513a 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2029,7 +2029,8 @@ static int do_umount(struct mount *mnt, int flags) * 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. + * what they covered. DCACHE_UNCOVERED tells a walk that found it earlier + * that the mounts are gone. The caller removes it for good afterwards. * * The caller may hold dentry->d_inode->i_rwsem. */ @@ -2047,7 +2048,10 @@ void __detach_mounts(struct dentry *dentry) return; /* the name goes first, what covered it goes second */ - d_drop(dentry); + spin_lock(&dentry->d_lock); + dentry->d_flags |= DCACHE_UNCOVERED; + __d_drop(dentry); + spin_unlock(&dentry->d_lock); event++; while (mp.node.next) { mnt = hlist_entry(mp.node.next, struct mount, mnt_mp_list); diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 4b1ff99608e0..673401db8c01 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -236,7 +236,8 @@ enum dentry_flags { DCACHE_PAR_LOOKUP = BIT(24), /* being looked up (with parent locked shared) */ DCACHE_DENTRY_CURSOR = BIT(25), DCACHE_NORCU = BIT(26), /* No RCU delay for freeing */ - DCACHE_PERSISTENT = BIT(27) + DCACHE_PERSISTENT = BIT(27), + DCACHE_UNCOVERED = BIT(28), /* lost its mounts on its way out */ }; #define DCACHE_MANAGED_DENTRY \ -- 2.53.0