1. Patchew
  2. linux
  3. View series

[PATCH v3] dcache: unpoison the inline name buffer in __d_alloc()

Drif Abdelmalek Mohamed Said posted 1 patch 5 days, 23 hours ago 
fs/dcache.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH v3] dcache: unpoison the inline name buffer in __d_alloc()
Posted by Drif Abdelmalek Mohamed Said 5 days, 23 hours ago 
syzbot reported:

    BUG: KMSAN: uninit-value in dentry_string_cmp fs/dcache.c:291 [inline]
    BUG: KMSAN: uninit-value in dentry_cmp fs/dcache.c:322 [inline]
    BUG: KMSAN: uninit-value in __d_lookup_rcu+0x37d/0x5e0 fs/dcache.c:2522

     dentry_string_cmp fs/dcache.c:291 [inline]
     dentry_cmp fs/dcache.c:322 [inline]
     __d_lookup_rcu+0x37d/0x5e0 fs/dcache.c:2522
     lookup_fast+0x194/0xa40 fs/namei.c:1854
     lookup_fast_for_open fs/namei.c:4545 [inline]
     open_last_lookups fs/namei.c:4579 [inline]
     path_openat+0x9ef/0x6540 fs/namei.c:4856
     do_file_open+0x2aa/0x680 fs/namei.c:4888
     do_sys_openat2+0x17c/0x390 fs/open.c:1395
     do_sys_open fs/open.c:1401 [inline]
     __do_sys_openat fs/open.c:1417 [inline]
     __se_sys_openat fs/open.c:1412 [inline]
     __x64_sys_openat+0x240/0x300 fs/open.c:1412
     x64_sys_call+0x2445/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:258
     do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
     do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

Uninit was stored to memory at:
     copy_name fs/dcache.c:3031 [inline]
     __d_move+0xd29/0x21f0 fs/dcache.c:3099
     d_move+0x71/0xf0 fs/dcache.c:3147
     vfs_rename+0x2619/0x2770 fs/namei.c:6085
     filename_renameat2+0xa59/0x1230 fs/namei.c:6188
     __do_sys_rename fs/namei.c:6232 [inline]
     __se_sys_rename+0xc5/0x5c0 fs/namei.c:6228
     __x64_sys_rename+0x78/0xb0 fs/namei.c:6228
     x64_sys_call+0x329/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:83
     do_syscall_x64 arch/x86/entry/syscall_64.c:63
     do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

Uninit was created at:
     slab_post_alloc_hook mm/slub.c:4617 [inline]
     slab_alloc_node mm/slub.c:4939 [inline]
     kmem_cache_alloc_lru_noprof+0x376/0x1230 mm/s
     __d_alloc+0x52/0x9f0 fs/dcache.c:1902
     d_alloc+0x57/0x300 fs/dcache.c:1981
     lookup_one_qstr_excl+0x19d/0x7a0 fs/namei.c:1806
     __start_renaming+0x341/0x850 fs/namei.c:3888
     filename_renameat2+0x625/0x1230 fs/namei.c:6163
     __do_sys_rename fs/namei.c:6232 [inline]
     __se_sys_rename+0xc5/0x5c0 fs/namei.c:6228
     __x64_sys_rename+0x78/0xb0 fs/namei.c:6228
     x64_sys_call+0x329/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:83
     do_syscall_x64 arch/x86/entry/syscall_64.c:63
     do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

The race is between a concurrent open() and rename() of the same path.

__d_alloc() only stores the name itself and its terminating NUL, so the
rest of the inline buffer (d_shortname, DNAME_INLINE_LEN bytes) is left
uninitialized.  copy_name(), called from rename(), copies that buffer as a
whole, so the uninitialized tail is propagated into the dentry that is
being moved.  Meanwhile __d_lookup_rcu(), called from open(), is an
optimistic lockless lookup: it checks d_name.hash_len first and leaves the
seqcount retry to its caller, so it can end up comparing against a dentry
whose name a rename is rewriting in place, using a stale (longer) length.
The comparison then runs past the terminating NUL and reads bytes of the
uninitialized tail, which KMSAN reports.

The read is harmless by design: it stays inside the buffer, the name is
still NUL-terminated, and the result is thrown away by the seqcount retry.
It is not specific to KMSAN either - with CONFIG_DCACHE_WORD_ACCESS
enabled the very same bytes are read by read_word_at_a_time(), which is
__no_sanitize_or_inline and therefore invisible to KMSAN.  KMSAN builds
only see the instrumented byte-at-a-time dentry_string_cmp() because
CONFIG_DCACHE_WORD_ACCESS is disabled when KMSAN is enabled on x86:

commit 7cf8f44a5a1c ("x86: fs: kmsan: disable CONFIG_DCACHE_WORD_ACCESS")

Zeroing the inline buffer would hide the report, but it would add a
memset() to a hot allocation path just to initialize bytes that are never
used as part of a name.  Instead, tell KMSAN the inline buffer is
initialized: kmsan_unpoison_memory() compiles to nothing unless
CONFIG_KMSAN is set, and doing it at allocation time is enough for every
dentry, because copy_name() and swap_names() copy the whole buffer and
thus propagate its shadow.

Reported-by: syzbot+7ff3adde89dd795ad4c4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7ff3adde89dd795ad4c4
Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>

Changes in v3:
- Annotate for KMSAN instead of zeroing, as suggested in review: the read
  is harmless, so unpoison the inline buffer in __d_alloc() with
  kmsan_unpoison_memory() (a no-op unless CONFIG_KMSAN) rather than adding
  a memset() to the dentry allocation path.
- Document why only KMSAN builds report this at all: with
  CONFIG_DCACHE_WORD_ACCESS the same read goes through
  read_word_at_a_time(), which KMSAN does not instrument.
- Rewrite the commit message; the previous one had several truncated
  lines.
---
 fs/dcache.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1b1a81f10da6..a66be85f9d01 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1916,6 +1916,10 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
 	 * be overwriting an internal NUL character
 	 */
 	dentry->d_shortname.string[DNAME_INLINE_LEN-1] = 0;
+
+	/* Racy __d_lookup_rcu() walk may read past the NUL; harmless */
+	kmsan_unpoison_memory(dentry->d_shortname.string, DNAME_INLINE_LEN);
+
 	if (unlikely(!name)) {
 		name = &slash_name;
 		dname = dentry->d_shortname.string;
-- 
2.43.0
Re: [PATCH v3] dcache: unpoison the inline name buffer in __d_alloc()
Posted by Jan Kara 3 days, 14 hours ago 
On Fri 18-09-26 23:42:04, Drif Abdelmalek Mohamed Said wrote:
> syzbot reported:
> 
>     BUG: KMSAN: uninit-value in dentry_string_cmp fs/dcache.c:291 [inline]
>     BUG: KMSAN: uninit-value in dentry_cmp fs/dcache.c:322 [inline]
>     BUG: KMSAN: uninit-value in __d_lookup_rcu+0x37d/0x5e0 fs/dcache.c:2522
> 
>      dentry_string_cmp fs/dcache.c:291 [inline]
>      dentry_cmp fs/dcache.c:322 [inline]
>      __d_lookup_rcu+0x37d/0x5e0 fs/dcache.c:2522
>      lookup_fast+0x194/0xa40 fs/namei.c:1854
>      lookup_fast_for_open fs/namei.c:4545 [inline]
>      open_last_lookups fs/namei.c:4579 [inline]
>      path_openat+0x9ef/0x6540 fs/namei.c:4856
>      do_file_open+0x2aa/0x680 fs/namei.c:4888
>      do_sys_openat2+0x17c/0x390 fs/open.c:1395
>      do_sys_open fs/open.c:1401 [inline]
>      __do_sys_openat fs/open.c:1417 [inline]
>      __se_sys_openat fs/open.c:1412 [inline]
>      __x64_sys_openat+0x240/0x300 fs/open.c:1412
>      x64_sys_call+0x2445/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:258
>      do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>      do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> Uninit was stored to memory at:
>      copy_name fs/dcache.c:3031 [inline]
>      __d_move+0xd29/0x21f0 fs/dcache.c:3099
>      d_move+0x71/0xf0 fs/dcache.c:3147
>      vfs_rename+0x2619/0x2770 fs/namei.c:6085
>      filename_renameat2+0xa59/0x1230 fs/namei.c:6188
>      __do_sys_rename fs/namei.c:6232 [inline]
>      __se_sys_rename+0xc5/0x5c0 fs/namei.c:6228
>      __x64_sys_rename+0x78/0xb0 fs/namei.c:6228
>      x64_sys_call+0x329/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:83
>      do_syscall_x64 arch/x86/entry/syscall_64.c:63
>      do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> Uninit was created at:
>      slab_post_alloc_hook mm/slub.c:4617 [inline]
>      slab_alloc_node mm/slub.c:4939 [inline]
>      kmem_cache_alloc_lru_noprof+0x376/0x1230 mm/s
>      __d_alloc+0x52/0x9f0 fs/dcache.c:1902
>      d_alloc+0x57/0x300 fs/dcache.c:1981
>      lookup_one_qstr_excl+0x19d/0x7a0 fs/namei.c:1806
>      __start_renaming+0x341/0x850 fs/namei.c:3888
>      filename_renameat2+0x625/0x1230 fs/namei.c:6163
>      __do_sys_rename fs/namei.c:6232 [inline]
>      __se_sys_rename+0xc5/0x5c0 fs/namei.c:6228
>      __x64_sys_rename+0x78/0xb0 fs/namei.c:6228
>      x64_sys_call+0x329/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:83
>      do_syscall_x64 arch/x86/entry/syscall_64.c:63
>      do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> The race is between a concurrent open() and rename() of the same path.
> 
> __d_alloc() only stores the name itself and its terminating NUL, so the
> rest of the inline buffer (d_shortname, DNAME_INLINE_LEN bytes) is left
> uninitialized.  copy_name(), called from rename(), copies that buffer as a
> whole, so the uninitialized tail is propagated into the dentry that is
> being moved.  Meanwhile __d_lookup_rcu(), called from open(), is an
> optimistic lockless lookup: it checks d_name.hash_len first and leaves the
> seqcount retry to its caller, so it can end up comparing against a dentry
> whose name a rename is rewriting in place, using a stale (longer) length.
> The comparison then runs past the terminating NUL and reads bytes of the
> uninitialized tail, which KMSAN reports.
> 
> The read is harmless by design: it stays inside the buffer, the name is
> still NUL-terminated, and the result is thrown away by the seqcount retry.
> It is not specific to KMSAN either - with CONFIG_DCACHE_WORD_ACCESS
> enabled the very same bytes are read by read_word_at_a_time(), which is
> __no_sanitize_or_inline and therefore invisible to KMSAN.  KMSAN builds
> only see the instrumented byte-at-a-time dentry_string_cmp() because
> CONFIG_DCACHE_WORD_ACCESS is disabled when KMSAN is enabled on x86:
> 
> commit 7cf8f44a5a1c ("x86: fs: kmsan: disable CONFIG_DCACHE_WORD_ACCESS")
> 
> Zeroing the inline buffer would hide the report, but it would add a
> memset() to a hot allocation path just to initialize bytes that are never
> used as part of a name.  Instead, tell KMSAN the inline buffer is
> initialized: kmsan_unpoison_memory() compiles to nothing unless
> CONFIG_KMSAN is set, and doing it at allocation time is enough for every
> dentry, because copy_name() and swap_names() copy the whole buffer and
> thus propagate its shadow.
> 
> Reported-by: syzbot+7ff3adde89dd795ad4c4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7ff3adde89dd795ad4c4
> Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>

The --- separator should be here. I think Christian can fix this up on
commit. Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Changes in v3:
> - Annotate for KMSAN instead of zeroing, as suggested in review: the read
>   is harmless, so unpoison the inline buffer in __d_alloc() with
>   kmsan_unpoison_memory() (a no-op unless CONFIG_KMSAN) rather than adding
>   a memset() to the dentry allocation path.
> - Document why only KMSAN builds report this at all: with
>   CONFIG_DCACHE_WORD_ACCESS the same read goes through
>   read_word_at_a_time(), which KMSAN does not instrument.
> - Rewrite the commit message; the previous one had several truncated
>   lines.
> ---
>  fs/dcache.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 1b1a81f10da6..a66be85f9d01 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1916,6 +1916,10 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
>  	 * be overwriting an internal NUL character
>  	 */
>  	dentry->d_shortname.string[DNAME_INLINE_LEN-1] = 0;
> +
> +	/* Racy __d_lookup_rcu() walk may read past the NUL; harmless */
> +	kmsan_unpoison_memory(dentry->d_shortname.string, DNAME_INLINE_LEN);
> +
>  	if (unlikely(!name)) {
>  		name = &slash_name;
>  		dname = dentry->d_shortname.string;
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.