| Message ID | b35a6f5ed4492fe971dfb03218e9b5b9abdd72f5.1388381026.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Dec 30, 2013 at 01:29:08PM +0800, Hu Tao wrote: > When cluster size is big enough it can lead offset overflow > in qcow2_alloc_clusters_at(). This patch fixes it. ping. and be more descriptive: The allocation each time is stopped at L2 table boundary(see handle_alloc()), so the possible maximum bytes could be 2^(cluster_bits - 3 + cluster_bits) so int is safe for cluster_bits<=17, unsafe otherwise. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > block/qcow2-refcount.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index c974abe..b3ebb7f 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -676,7 +676,12 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, > BDRVQcowState *s = bs->opaque; > uint64_t cluster_index; > uint64_t old_free_cluster_index; > - int i, refcount, ret; > + uint64_t i; > + int refcount, ret; > + > + if (nb_clusters <= 0) { > + return 0; > + } > > /* Check how many clusters there are free */ > cluster_index = offset >> s->cluster_bits; > -- > 1.7.11.7 >
On 30.12.2013 06:29, Hu Tao wrote: > When cluster size is big enough it can lead offset overflow > in qcow2_alloc_clusters_at(). This patch fixes it. > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > block/qcow2-refcount.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index c974abe..b3ebb7f 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -676,7 +676,12 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, > BDRVQcowState *s = bs->opaque; > uint64_t cluster_index; > uint64_t old_free_cluster_index; > - int i, refcount, ret; > + uint64_t i; > + int refcount, ret; > + > + if (nb_clusters <= 0) { > + return 0; I think I'd rather return -EINVAL here. > + } > > /* Check how many clusters there are free */ > cluster_index = offset >> s->cluster_bits;
Am 19.01.2014 um 17:12 hat Max Reitz geschrieben: > On 30.12.2013 06:29, Hu Tao wrote: > >When cluster size is big enough it can lead offset overflow > >in qcow2_alloc_clusters_at(). This patch fixes it. > > > >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > >--- > > block/qcow2-refcount.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > >diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > >index c974abe..b3ebb7f 100644 > >--- a/block/qcow2-refcount.c > >+++ b/block/qcow2-refcount.c > >@@ -676,7 +676,12 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, > > BDRVQcowState *s = bs->opaque; > > uint64_t cluster_index; > > uint64_t old_free_cluster_index; > >- int i, refcount, ret; > >+ uint64_t i; > >+ int refcount, ret; > >+ > >+ if (nb_clusters <= 0) { > >+ return 0; > > I think I'd rather return -EINVAL here. In fact, I think return 0 is fine for nb_clusters == 0, and we should assert(nb_clusters >= 0). Kevin
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index c974abe..b3ebb7f 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -676,7 +676,12 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset, BDRVQcowState *s = bs->opaque; uint64_t cluster_index; uint64_t old_free_cluster_index; - int i, refcount, ret; + uint64_t i; + int refcount, ret; + + if (nb_clusters <= 0) { + return 0; + } /* Check how many clusters there are free */ cluster_index = offset >> s->cluster_bits;
When cluster size is big enough it can lead offset overflow in qcow2_alloc_clusters_at(). This patch fixes it. Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- block/qcow2-refcount.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)