| Message ID | 1484579832-18589-1-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On Mon, 01/16 16:17, Peter Lieven wrote: > nb_cls_shrunk in iscsi_allocmap_update can become -1 if the > request starts and ends within the same cluster. This results > in passing -1 to bitmap_set and bitmap_clear and they don't > handle negative values properly. In the end this leads to data > corruption. > > Fixes: e1123a3b40a1a9a625a29c8ed4debb7e206ea690 > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/iscsi.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 6aeeb9e..1860f1b 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -499,14 +499,18 @@ iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num, > if (allocated) { > bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded); > } else { > - bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); > + if (nb_cls_shrunk > 0) { > + bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); > + } > } > > if (iscsilun->allocmap_valid == NULL) { > return; > } > if (valid) { > - bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); > + if (nb_cls_shrunk > 0) { > + bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); > + } > } else { > bitmap_clear(iscsilun->allocmap_valid, cl_num_expanded, > nb_cls_expanded); > -- > 1.9.1 > > It's probably a good idea to add assertions parameter in bitmap_*. Reviewed-by: Fam Zheng <famz@redhat.com>
Am 17.01.2017 um 12:28 schrieb Fam Zheng: > On Mon, 01/16 16:17, Peter Lieven wrote: >> nb_cls_shrunk in iscsi_allocmap_update can become -1 if the >> request starts and ends within the same cluster. This results >> in passing -1 to bitmap_set and bitmap_clear and they don't >> handle negative values properly. In the end this leads to data >> corruption. >> >> Fixes: e1123a3b40a1a9a625a29c8ed4debb7e206ea690 >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> block/iscsi.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/block/iscsi.c b/block/iscsi.c >> index 6aeeb9e..1860f1b 100644 >> --- a/block/iscsi.c >> +++ b/block/iscsi.c >> @@ -499,14 +499,18 @@ iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num, >> if (allocated) { >> bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded); >> } else { >> - bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); >> + if (nb_cls_shrunk > 0) { >> + bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); >> + } >> } >> >> if (iscsilun->allocmap_valid == NULL) { >> return; >> } >> if (valid) { >> - bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); >> + if (nb_cls_shrunk > 0) { >> + bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); >> + } >> } else { >> bitmap_clear(iscsilun->allocmap_valid, cl_num_expanded, >> nb_cls_expanded); >> -- >> 1.9.1 >> >> > It's probably a good idea to add assertions parameter in bitmap_*. yes, i will send a follow-up patch. Peter
On 16/01/2017 16:17, Peter Lieven wrote: > nb_cls_shrunk in iscsi_allocmap_update can become -1 if the > request starts and ends within the same cluster. This results > in passing -1 to bitmap_set and bitmap_clear and they don't > handle negative values properly. In the end this leads to data > corruption. > > Fixes: e1123a3b40a1a9a625a29c8ed4debb7e206ea690 > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/iscsi.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Queued, thanks. Paolo
diff --git a/block/iscsi.c b/block/iscsi.c index 6aeeb9e..1860f1b 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -499,14 +499,18 @@ iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num, if (allocated) { bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded); } else { - bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); + if (nb_cls_shrunk > 0) { + bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk); + } } if (iscsilun->allocmap_valid == NULL) { return; } if (valid) { - bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); + if (nb_cls_shrunk > 0) { + bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk); + } } else { bitmap_clear(iscsilun->allocmap_valid, cl_num_expanded, nb_cls_expanded);
nb_cls_shrunk in iscsi_allocmap_update can become -1 if the request starts and ends within the same cluster. This results in passing -1 to bitmap_set and bitmap_clear and they don't handle negative values properly. In the end this leads to data corruption. Fixes: e1123a3b40a1a9a625a29c8ed4debb7e206ea690 Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven <pl@kamp.de> --- block/iscsi.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)