linux-kernel - Re: [PATCH] bcachefs: Fix kmalloc bug in __snapshot_t

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 for Android: free password hash cracker in your pocket

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

Message-ID: <goqpw7dkw4zadoutq3ekp3cd3ve2y4ufci5etap3lmwh7rbbxp@v4vsiz6p3dcg>
Date: Tue, 25 Jun 2024 20:51:04 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Pei Li <peili.dev@...il.com>
Cc: Brian Foster <bfoster@...hat.com>, linux-bcachefs@...r.kernel.org, 
	linux-kernel@...r.kernel.org, skhan@...uxfoundation.org, 
	linux-kernel-mentees@...ts.linuxfoundation.org, syzkaller-bugs@...glegroups.com, 
	syzbot+770e99b65e26fa023ab1@...kaller.appspotmail.com
Subject: Re: [PATCH] bcachefs: Fix kmalloc bug in __snapshot_t_mut

On Tue, Jun 25, 2024 at 05:39:56PM -0700, Pei Li wrote:
> When allocating too huge a snapshot table, we should fail gracefully
> in __snapshot_t_mut() instead of fail in kmalloc().
> 
> Reported-by: syzbot+770e99b65e26fa023ab1@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=770e99b65e26fa023ab1
> Tested-by: syzbot+770e99b65e26fa023ab1@...kaller.appspotmail.com
> Signed-off-by: Pei Li <peili.dev@...il.com>
> ---
> Syzbot reported the following warning in kmalloc().
> 
> bcachefs (loop0): mounting version 1.7: mi_btree_bitmap opts=metadata_checksum=crc64,data_checksum=xxhash,compression=gzip,str_hash=crc64,nojournal_transaction_names
> bcachefs (loop0): recovering from clean shutdown, journal seq 7
> bcachefs (loop0): alloc_read... done
> bcachefs (loop0): stripes_read... done
> bcachefs (loop0): snapshots_read...
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 5078 at mm/util.c:649 kvmalloc_node_noprof+0x17a/0x190 mm/util.c:649
> 
> We are passing a huge new_bytes (greater than INT_MAX) to kmalloc().
> 
> This is likely caused by either we run out of snapshot entries when
> calculating the size of snapshot table, or an invalid bkey was read.
> 
> Instead of failing at kmalloc(), handle this error when a large size of
> memory is going to be requested and return NULL directly.
> 
> syzbot has tested the proposed patch and the reproducer did not trigger
> any issue.
> 
> Tested on:
> 
> commit:         55027e68 Merge tag 'input-for-v6.10-rc5' of git://git...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=10f3a389980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=bf5def5af476d39a
> dashboard link: https://syzkaller.appspot.com/bug?extid=770e99b65e26fa023ab1
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=170174c1980000
> 
> Note: testing is done by a robot and is best-effort only.

Thanks, nice fix - applied

> ---
>  fs/bcachefs/snapshot.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/bcachefs/snapshot.c b/fs/bcachefs/snapshot.c
> index 629900a5e641..e3303aea8b29 100644
> --- a/fs/bcachefs/snapshot.c
> +++ b/fs/bcachefs/snapshot.c
> @@ -168,6 +168,9 @@ static noinline struct snapshot_t *__snapshot_t_mut(struct bch_fs *c, u32 id)
>  	size_t new_bytes = kmalloc_size_roundup(struct_size(new, s, idx + 1));
>  	size_t new_size = (new_bytes - sizeof(*new)) / sizeof(new->s[0]);
>  
> +	if (unlikely(new_bytes > INT_MAX))
> +		return NULL;
> +
>  	new = kvzalloc(new_bytes, GFP_KERNEL);
>  	if (!new)
>  		return NULL;
> 
> ---
> base-commit: c13320499ba0efd93174ef6462ae8a7a2933f6e7
> change-id: 20240625-bug3-9660c6b76f20
> 
> Best regards,
> -- 
> Pei Li <peili.dev@...il.com>
>