linux-kernel - Re: [PATCH] autofs: fix null deref in autofs_fill

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <4fcf49456c32087f5306e84c4a8df5b2bd9f4146.camel@themaw.net>
Date:   Tue, 14 Nov 2023 12:25:35 +0800
From:   Ian Kent <raven@...maw.net>
To:     Edward Adam Davis <eadavis@...com>,
        syzbot+662f87a8ef490f45fa64@...kaller.appspotmail.com
Cc:     autofs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] autofs: fix null deref in autofs_fill_super

On Tue, 2023-11-14 at 11:52 +0800, Edward Adam Davis wrote:
> [Syz logs]
> KASAN: null-ptr-deref in range [0x0000000000000000-
> 0x0000000000000007]
> CPU: 0 PID: 5098 Comm: syz-executor.0 Not tainted 6.6.0-syzkaller-
> 15601-g4bbdb725a36b #0
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 10/09/2023
> RIP: 0010:autofs_fill_super+0x47d/0xb50 fs/autofs/inode.c:334
> 
> [pid  5095] mount(NULL, "./file1", "autofs", 0,
> "fd=0x0000000000000000") = -1 ENOMEM (Cannot allocate memory)
> 
> [Analysis]
> autofs_get_inode() will return null, when memory cannot be allocated.
> 
> [Fix]
> Confirm that root_inde is not null before using it.
> 
> Reported-and-tested-by:
> syzbot+662f87a8ef490f45fa64@...kaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> ---
>  fs/autofs/inode.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
> index a5083d447a62..f2e89a444edf 100644
> --- a/fs/autofs/inode.c
> +++ b/fs/autofs/inode.c
> @@ -331,6 +331,9 @@ static int autofs_fill_super(struct super_block
> *s, struct fs_context *fc)
>                 goto fail;
>  
>         root_inode = autofs_get_inode(s, S_IFDIR | 0755);
> +       if (!root_inode)
> +               goto fail;

Yes, I think this is the only thing it could be.

There's one small problem though, it leaks the dentry info. ino,
allocated just above. I think this should goto label fail_ino instead.

Note that once the root dentry is allocated then the ino struct will
be freed when the dentry is freed so ino doesn't need to be freed.

Ian