1. Patchew
  2. linux
  3. View series

[PATCH] hfsplus: fix uninit-value in copy_name

Edward Adam Davis posted 1 patch 1 year, 8 months ago 
fs/hfsplus/xattr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hfsplus: fix uninit-value in copy_name
Posted by Edward Adam Davis 1 year, 8 months ago 
[syzbot reported]
BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
 sized_strscpy+0xc4/0x160
 copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
 hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
 vfs_listxattr fs/xattr.c:493 [inline]
 listxattr+0x1f3/0x6b0 fs/xattr.c:840
 path_listxattr fs/xattr.c:864 [inline]
 __do_sys_listxattr fs/xattr.c:876 [inline]
 __se_sys_listxattr fs/xattr.c:873 [inline]
 __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
 x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Uninit was created at:
 slab_post_alloc_hook mm/slub.c:3877 [inline]
 slab_alloc_node mm/slub.c:3918 [inline]
 kmalloc_trace+0x57b/0xbe0 mm/slub.c:4065
 kmalloc include/linux/slab.h:628 [inline]
 hfsplus_listxattr+0x4cc/0x1a50 fs/hfsplus/xattr.c:699
 vfs_listxattr fs/xattr.c:493 [inline]
 listxattr+0x1f3/0x6b0 fs/xattr.c:840
 path_listxattr fs/xattr.c:864 [inline]
 __do_sys_listxattr fs/xattr.c:876 [inline]
 __se_sys_listxattr fs/xattr.c:873 [inline]
 __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
 x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
[Fix]
When allocating memory to strbuf, initialize memory to 0.

Reported-and-tested-by: syzbot+efde959319469ff8d4d7@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/hfsplus/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 9c9ff6b8c6f7..858029b1c173 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -698,7 +698,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
 		return err;
 	}
 
-	strbuf = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
+	strbuf = kzalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
 			XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL);
 	if (!strbuf) {
 		res = -ENOMEM;
-- 
2.43.0
Re: [PATCH] hfsplus: fix uninit-value in copy_name
Posted by Kees Cook 1 year, 7 months ago 
On Tue, 21 May 2024 13:21:46 +0800, Edward Adam Davis wrote:
> [syzbot reported]
> BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
>  sized_strscpy+0xc4/0x160
>  copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
>  hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
>  vfs_listxattr fs/xattr.c:493 [inline]
>  listxattr+0x1f3/0x6b0 fs/xattr.c:840
>  path_listxattr fs/xattr.c:864 [inline]
>  __do_sys_listxattr fs/xattr.c:876 [inline]
>  __se_sys_listxattr fs/xattr.c:873 [inline]
>  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
>  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> [...]

I've taken some security-related hfsplus stuff before, so:

Applied to for-next/hardening, thanks!

[1/1] hfsplus: fix uninit-value in copy_name
      https://git.kernel.org/kees/c/974a00974829

Take care,

-- 
Kees Cook
Re: [PATCH] hfsplus: fix uninit-value in copy_name
Posted by Christian Brauner 1 year, 7 months ago 
On Fri, Jul 05, 2024 at 02:20:54PM GMT, Kees Cook wrote:
> On Tue, 21 May 2024 13:21:46 +0800, Edward Adam Davis wrote:
> > [syzbot reported]
> > BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
> >  sized_strscpy+0xc4/0x160
> >  copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
> >  hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
> >  vfs_listxattr fs/xattr.c:493 [inline]
> >  listxattr+0x1f3/0x6b0 fs/xattr.c:840
> >  path_listxattr fs/xattr.c:864 [inline]
> >  __do_sys_listxattr fs/xattr.c:876 [inline]
> >  __se_sys_listxattr fs/xattr.c:873 [inline]
> >  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
> >  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
> >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> >  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
> >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > 
> > [...]
> 
> I've taken some security-related hfsplus stuff before, so:

It's in #vfs.fixes to go out with the next vfs fixes pr.
Re: [PATCH] hfsplus: fix uninit-value in copy_name
Posted by Kees Cook 1 year, 7 months ago 
On Sat, Jul 06, 2024 at 09:20:30AM +0200, Christian Brauner wrote:
> On Fri, Jul 05, 2024 at 02:20:54PM GMT, Kees Cook wrote:
> > On Tue, 21 May 2024 13:21:46 +0800, Edward Adam Davis wrote:
> > > [syzbot reported]
> > > BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
> > >  sized_strscpy+0xc4/0x160
> > >  copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
> > >  hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
> > >  vfs_listxattr fs/xattr.c:493 [inline]
> > >  listxattr+0x1f3/0x6b0 fs/xattr.c:840
> > >  path_listxattr fs/xattr.c:864 [inline]
> > >  __do_sys_listxattr fs/xattr.c:876 [inline]
> > >  __se_sys_listxattr fs/xattr.c:873 [inline]
> > >  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
> > >  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
> > >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > >  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
> > >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > > 
> > > [...]
> > 
> > I've taken some security-related hfsplus stuff before, so:
> 
> It's in #vfs.fixes to go out with the next vfs fixes pr.

Ah, thanks! I've dropped it from my tree now.

-- 
Kees Cook
Re: [PATCH] hfsplus: fix uninit-value in copy_name
Posted by Dmitry Vyukov 1 year, 7 months ago 
On Sat, 6 Jul 2024 at 09:20, Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Jul 05, 2024 at 02:20:54PM GMT, Kees Cook wrote:
> > On Tue, 21 May 2024 13:21:46 +0800, Edward Adam Davis wrote:
> > > [syzbot reported]
> > > BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
> > >  sized_strscpy+0xc4/0x160
> > >  copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
> > >  hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
> > >  vfs_listxattr fs/xattr.c:493 [inline]
> > >  listxattr+0x1f3/0x6b0 fs/xattr.c:840
> > >  path_listxattr fs/xattr.c:864 [inline]
> > >  __do_sys_listxattr fs/xattr.c:876 [inline]
> > >  __se_sys_listxattr fs/xattr.c:873 [inline]
> > >  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
> > >  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
> > >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > >  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
> > >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > >
> > > [...]
> >
> > I've taken some security-related hfsplus stuff before, so:
>
> It's in #vfs.fixes to go out with the next vfs fixes pr.

Oops, sorry for creating a mess.
It's just really hard to understand a random patch status.
Re: [PATCH] hfsplus: fix uninit-value in copy_name
Posted by Dmitry Vyukov 1 year, 7 months ago 
On Tue, 21 May 2024 at 07:28, 'Edward Adam Davis' via syzkaller-bugs
<syzkaller-bugs@googlegroups.com> wrote:
>
> [syzbot reported]
> BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
>  sized_strscpy+0xc4/0x160
>  copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
>  hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
>  vfs_listxattr fs/xattr.c:493 [inline]
>  listxattr+0x1f3/0x6b0 fs/xattr.c:840
>  path_listxattr fs/xattr.c:864 [inline]
>  __do_sys_listxattr fs/xattr.c:876 [inline]
>  __se_sys_listxattr fs/xattr.c:873 [inline]
>  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
>  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Uninit was created at:
>  slab_post_alloc_hook mm/slub.c:3877 [inline]
>  slab_alloc_node mm/slub.c:3918 [inline]
>  kmalloc_trace+0x57b/0xbe0 mm/slub.c:4065
>  kmalloc include/linux/slab.h:628 [inline]
>  hfsplus_listxattr+0x4cc/0x1a50 fs/hfsplus/xattr.c:699
>  vfs_listxattr fs/xattr.c:493 [inline]
>  listxattr+0x1f3/0x6b0 fs/xattr.c:840
>  path_listxattr fs/xattr.c:864 [inline]
>  __do_sys_listxattr fs/xattr.c:876 [inline]
>  __se_sys_listxattr fs/xattr.c:873 [inline]
>  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
>  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [Fix]
> When allocating memory to strbuf, initialize memory to 0.
>
> Reported-and-tested-by: syzbot+efde959319469ff8d4d7@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  fs/hfsplus/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> index 9c9ff6b8c6f7..858029b1c173 100644
> --- a/fs/hfsplus/xattr.c
> +++ b/fs/hfsplus/xattr.c
> @@ -698,7 +698,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
>                 return err;
>         }
>
> -       strbuf = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
> +       strbuf = kzalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
>                         XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL);
>         if (!strbuf) {
>                 res = -ENOMEM;

Hi Edward,

Was this ever merged anywhere? I still don't see it upstream.
Re: [PATCH] hfsplus: fix uninit-value in copy_name
Posted by Edward Adam Davis 1 year, 7 months ago 
On Thu, 4 Jul 2024 15:36:37 +0200, Dmitry Vyukov wrote:
> > [syzbot reported]
> > BUG: KMSAN: uninit-value in sized_strscpy+0xc4/0x160
> >  sized_strscpy+0xc4/0x160
> >  copy_name+0x2af/0x320 fs/hfsplus/xattr.c:411
> >  hfsplus_listxattr+0x11e9/0x1a50 fs/hfsplus/xattr.c:750
> >  vfs_listxattr fs/xattr.c:493 [inline]
> >  listxattr+0x1f3/0x6b0 fs/xattr.c:840
> >  path_listxattr fs/xattr.c:864 [inline]
> >  __do_sys_listxattr fs/xattr.c:876 [inline]
> >  __se_sys_listxattr fs/xattr.c:873 [inline]
> >  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
> >  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
> >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> >  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
> >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >
> > Uninit was created at:
> >  slab_post_alloc_hook mm/slub.c:3877 [inline]
> >  slab_alloc_node mm/slub.c:3918 [inline]
> >  kmalloc_trace+0x57b/0xbe0 mm/slub.c:4065
> >  kmalloc include/linux/slab.h:628 [inline]
> >  hfsplus_listxattr+0x4cc/0x1a50 fs/hfsplus/xattr.c:699
> >  vfs_listxattr fs/xattr.c:493 [inline]
> >  listxattr+0x1f3/0x6b0 fs/xattr.c:840
> >  path_listxattr fs/xattr.c:864 [inline]
> >  __do_sys_listxattr fs/xattr.c:876 [inline]
> >  __se_sys_listxattr fs/xattr.c:873 [inline]
> >  __x64_sys_listxattr+0x16b/0x2f0 fs/xattr.c:873
> >  x64_sys_call+0x2ba0/0x3b50 arch/x86/include/generated/asm/syscalls_64.h:195
> >  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> >  do_syscall_64+0xcf/0x1e0 arch/x86/entry/common.c:83
> >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > [Fix]
> > When allocating memory to strbuf, initialize memory to 0.
> >
> > Reported-and-tested-by: syzbot+efde959319469ff8d4d7@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> >  fs/hfsplus/xattr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> > index 9c9ff6b8c6f7..858029b1c173 100644
> > --- a/fs/hfsplus/xattr.c
> > +++ b/fs/hfsplus/xattr.c
> > @@ -698,7 +698,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
> >                 return err;
> >         }
> >
> > -       strbuf = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
> > +       strbuf = kzalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
> >                         XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL);
> >         if (!strbuf) {
> >                 res = -ENOMEM;
> 
> Hi Edward,
> 
> Was this ever merged anywhere? I still don't see it upstream.
Yes, me too.

--
Edward

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.