fs/dcache.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
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 happens between a concurrent open() and rename() of the same
path. __d_alloc() initializes only the name itsel
of the inline buffer; the tail in between is left uninitialized.
copy_name(), called from rename(), copies the enti
including that uninitialized tail - into the moved dentry. Meanwhile
__d_lookup_rcu(), called from open(), is an optimi
it checks d_name.hash_len first and leaves the seqcount retry to its
caller, so a lookup of the old (longer) name racin
compares with a stale length. While copy_name() is rewriting the name
in place, the walker can transiently step past the
terminating NUL and read bytes from the uninitialized tail, which KMSAN
reports.
The race is benign by design - the read stays in bounds and the lookup
result is discarded by the seqcount retry - but th
is real. Fix it by zeroing the entire inline buffer in __d_alloc(),
so every byte a racy walker can touch is defined;
past a mid-rewrite name now simply stops at a NUL.
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>
---
v2:
- Screwed up by using memcpy in v1, fixed it here by using memset, only change
fs/dcache.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 1b1a81f10da6..730571e92af8 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1910,12 +1910,17 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
return NULL;
/*
- * We guarantee that the inline name is always NUL-terminated.
- * This way the memcpy() done by the name switching in rename
- * will still always have a NUL at the end, even if we might
- * be overwriting an internal NUL character
+ * Fully initialize the inline name buffer. copy_name() and
+ * swap_names() copy d_shortname in its entirety, so any
+ * uninitialized tail would propagate to the other dentry, and
+ * __d_lookup_rcu() may transiently read any byte of the inline
+ * name while rename() rewrites it in place.
+ *
+ * This also keeps the inline name NUL-terminated: the name
+ * switching in rename will still always have a NUL at the end,
+ * even if we might be overwriting an internal NUL character.
*/
- dentry->d_shortname.string[DNAME_INLINE_LEN-1] = 0;
+ memset(dentry->d_shortname.string, 0, DNAME_INLINE_LEN);
if (unlikely(!name)) {
name = &slash_name;
dname = dentry->d_shortname.string;
--
2.43.0
On Mon 14-09-26 20:29:26, 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 happens between a concurrent open() and rename() of the same
> path. __d_alloc() initializes only the name itsel
> of the inline buffer; the tail in between is left uninitialized.
> copy_name(), called from rename(), copies the enti
> including that uninitialized tail - into the moved dentry. Meanwhile
> __d_lookup_rcu(), called from open(), is an optimi
> it checks d_name.hash_len first and leaves the seqcount retry to its
> caller, so a lookup of the old (longer) name racin
> compares with a stale length. While copy_name() is rewriting the name
> in place, the walker can transiently step past the
> terminating NUL and read bytes from the uninitialized tail, which KMSAN
> reports.
>
> The race is benign by design - the read stays in bounds and the lookup
> result is discarded by the seqcount retry - but th
> is real. Fix it by zeroing the entire inline buffer in __d_alloc(),
> so every byte a racy walker can touch is defined;
> past a mid-rewrite name now simply stops at a NUL.
>
> 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>
Firstly, the lines in the changelog are weirdly truncated so it is barely
readable. Please fix that. Secondly, I don't think it is reasonable to
impose additional cost of zeroing on this hot path only to fix harmless
KMSAN warning. So if we can somehow annotate this to silence KMSAN then
that would be a way to go in my opinion.
Honza
> ---
> v2:
> - Screwed up by using memcpy in v1, fixed it here by using memset, only change
> fs/dcache.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 1b1a81f10da6..730571e92af8 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1910,12 +1910,17 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
> return NULL;
>
> /*
> - * We guarantee that the inline name is always NUL-terminated.
> - * This way the memcpy() done by the name switching in rename
> - * will still always have a NUL at the end, even if we might
> - * be overwriting an internal NUL character
> + * Fully initialize the inline name buffer. copy_name() and
> + * swap_names() copy d_shortname in its entirety, so any
> + * uninitialized tail would propagate to the other dentry, and
> + * __d_lookup_rcu() may transiently read any byte of the inline
> + * name while rename() rewrites it in place.
> + *
> + * This also keeps the inline name NUL-terminated: the name
> + * switching in rename will still always have a NUL at the end,
> + * even if we might be overwriting an internal NUL character.
> */
> - dentry->d_shortname.string[DNAME_INLINE_LEN-1] = 0;
> + memset(dentry->d_shortname.string, 0, 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
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
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
© 2016 - 2026 Red Hat, Inc.