Re: [Qemu-devel] [Qemu-stable] [PATCH] block/iscsi: avoid data corruption with cache=writeback

[Peter Lieven]

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: address@hidden
> > Signed-off-by: Peter Lieven <address@hidden>
> > ---
> >   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