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

[Peter Lieven <pl@kamp.de>]

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