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: [email protected] > Closes: https://syzkaller.appspot.com/bug?extid=770e99b65e26fa023ab1 > Tested-by: [email protected] > Signed-off-by: Pei Li <[email protected]> > --- > 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 <[email protected]> >
