Re: [Qemu-block] [PATCH v2 2/3] qcow2: Don't allow overflow during cluster allocation

2018-02-22 Thread Eric Blake

On 02/22/2018 04:29 AM, Alberto Garcia wrote:

On Thu 22 Feb 2018 12:39:52 AM CET, Eric Blake wrote:

  free_in_cluster = s->cluster_size - offset_into_cluster(s, offset);
  do {
  if (!offset || free_in_cluster < size) {
-int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
+int64_t new_cluster;
+
+new_cluster = alloc_clusters_noref(bs, s->cluster_size,
+   (1ULL << s->csize_shift) - 1);


(1ULL << s->csize_shift) - 1) is the same as s->cluster_offset_mask, but
I guess it's confusing to use that here, so your approach looks
appropriate.


Actually, s->cluster_offset_mask fits better - we want to ensure that 
the allocated cluster fits within the mask!  I'll adjust on respin.




Reviewed-by: Alberto Garcia 


Thanks for bearing with me.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



Re: [Qemu-block] [PATCH v2 2/3] qcow2: Don't allow overflow during cluster allocation

2018-02-22 Thread Alberto Garcia
On Thu 22 Feb 2018 12:39:52 AM CET, Eric Blake wrote:
>  free_in_cluster = s->cluster_size - offset_into_cluster(s, offset);
>  do {
>  if (!offset || free_in_cluster < size) {
> -int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
> +int64_t new_cluster;
> +
> +new_cluster = alloc_clusters_noref(bs, s->cluster_size,
> +   (1ULL << s->csize_shift) - 1);

(1ULL << s->csize_shift) - 1) is the same as s->cluster_offset_mask, but
I guess it's confusing to use that here, so your approach looks
appropriate.

Reviewed-by: Alberto Garcia 

Berto