[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qcow2-cluster: Fix integer left shift error in qcow2_alloc_c
Re: [PATCH] qcow2-cluster: Fix integer left shift error in qcow2_alloc_cluster_link_l2()
Wed, 05 Aug 2020 16:32:48 +0200
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu)
On Wed 05 Aug 2020 04:16:57 PM CEST, Kevin Wolf wrote:
>> nb_clusters is an int and there are more cases of
>> nb_clusters << s->cluster_bits
>> I can see at least these: handle_alloc(), qcow2_free_any_clusters(),
> Actuallyx, handle_alloc() and everything that comes from it should be
> fine. It has a uint64_t nb_clusters locally and limits it:
> nb_clusters = MIN(nb_clusters, INT_MAX >> s->cluster_bits);
INT_MAX replaced with BDRV_REQUEST_MAX_BYTES in my subcluster allocation
series, so it should still be fine.
> The problematic request that causes the crash comes actually from
> qcow2_co_truncate(). It limits it only to s->l2_slice_size, which can
> be larger than that, but will be at most 256k (= 2 MB /
> cow_end.offset will get a wraparound then, too. This is harmless
> because cow_end.nb_bytes = 0, so the offset will be ignored anyway.
In that one nb_clusters is actually int64_t so there's no wraparound.
> I think the proper fix to be made in the 5.2 release cycle would revert
> this one and instead fix the limit in qcow2_co_truncate().
> But this one is good enough as a band-aid for 5.1.
The other one is just as simple, no? This line in the while() loop in
nb_clusters = MIN(nb_clusters, BDRV_REQUEST_MAX_BYTES >> s->cluster_bits);