linux-kernel - Re: [PATCH] fs/exfat: resolve memory leak from exfat_create_upcase

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240915070546.GE2825852@ZenIV>
Date: Sun, 15 Sep 2024 08:05:46 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Daniel Yang <danielyangkang@...il.com>
Cc: Namjae Jeon <linkinjeon@...nel.org>,
	Sungjong Seo <sj1557.seo@...sung.com>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	syzbot+e1c69cadec0f1a078e3d@...kaller.appspotmail.com
Subject: Re: [PATCH] fs/exfat: resolve memory leak from
 exfat_create_upcase_table()

On Sat, Sep 14, 2024 at 11:44:03PM -0700, Daniel Yang wrote:
>     If exfat_load_upcase_table reaches end and returns -EINVAL,
>     allocated memory doesn't get freed and while
>     exfat_load_default_upcase_table allocates more memory, leading to a    
>     memory leak.
>     
>     Here's link to syzkaller crash report illustrating this issue:
>     https://syzkaller.appspot.com/text?tag=CrashReport&x=1406c201980000
> 
> Signed-off-by: Daniel Yang <danielyangkang@...il.com>
> Reported-by: syzbot+e1c69cadec0f1a078e3d@...kaller.appspotmail.com
> ---
>  fs/exfat/nls.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c
> index afdf13c34..ec69477d0 100644
> --- a/fs/exfat/nls.c
> +++ b/fs/exfat/nls.c
> @@ -699,6 +699,7 @@ static int exfat_load_upcase_table(struct super_block *sb,
>  
>  	exfat_err(sb, "failed to load upcase table (idx : 0x%08x, chksum : 0x%08x, utbl_chksum : 0x%08x)",
>  		  index, chksum, utbl_checksum);
> +	exfat_free_upcase_table(sbi);
>  	return -EINVAL;
>  }

	Interesting...  How does the mainline manage to avoid the
call of exfat_kill_sb(), which should call_rcu() delayed_free(), which
calls exfat_free_upcase_table()?

	Could you verify that your reproducer does *NOT* hit that
callchain?  AFAICS, the only caller of exfat_load_upcase_table()
is exfat_create_upcase_table(), called by __exfat_fill_super(),
called by exfat_fill_super(), passed as callback to get_tree_bdev().
And if that's the case, ->kill_sb() should be called on failure and
with non-NULL ->s_fs_info...

	Something odd is going on there.