When allocating too huge a snapshot table, we should fail gracefully
in __snapshot_t_mut() instead of fail in kmalloc().

Reported-by: syzbot+770e99b65e26fa023ab1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=770e99b65e26fa023ab1
Tested-by: syzbot+770e99b65e26fa023ab1@syzkaller.appspotmail.com
Signed-off-by: Pei Li <peili.dev@gmail.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.
---
 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@gmail.com>

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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=770e99b65e26fa023ab1
> Tested-by: syzbot+770e99b65e26fa023ab1@syzkaller.appspotmail.com
> Signed-off-by: Pei Li <peili.dev@gmail.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@gmail.com>
>