Re: [Qemu-devel] [PATCH 1/7] block: only force IO completion in .bdrv_truncate if we are shrinking

[Peter Lieven]

Am 07.03.2013 um 11:00 schrieb Kevin Wolf <address@hidden>:

> Am 07.03.2013 um 10:25 hat Peter Lieven geschrieben:
>> 
>> Am 07.03.2013 um 10:22 schrieb Kevin Wolf <address@hidden>:
>> 
>>> Am 07.03.2013 um 10:16 hat Peter Lieven geschrieben:
>>>>>> If bs->growable is 1 for all drivers, whats the fix status of 
>>>>>> CVE-2008-0928? This
>>>>>> flag was introduced as a fix for this problem.
>>>>>> 
>>>>>> bdrv_check_byte_request() does nothing useful if bs->growable is 1.
>>>>> 
>>>>> Don't ignore the difference between bdrv_open() and bdrv_file_open().
>>>>> Typically you have two BDSes: On top there is e.g. a qcow2 BDS that is
>>>>> opened through bdrv_open() and has bs->growable = false. Its bs->file is
>>>>> using the file protocol (raw-posix driver) and opened by
>>>>> bdrv_file_open(). This one has bs->file->growable = true so that qcow2
>>>>> can write to newly allocated areas without calling bdrv_truncate()
>>>>> first.
>>>> 
>>>> Sorry, I have to admin I am little confused by what is happening in 
>>>> bdrv_open().
>>>> 
>>>> However, what I can say is that bs->growable is 1 for an iSCSI backed
>>>> harddrive and I wonder how this can happen if bdrv_file_open is not used 
>>>> for
>>>> opening it because that is the only place where bs->growable is set to 1.
>>>> 
>>>> cmdline:
>>>> x86_64-softmmu/qemu-system-x86_64 -k de -enable-kvm -m 1024 -drive 
>>>> if=virtio,file=iscsi://172.21.200.31/iqn.2001-05.com.equallogic:0-8a0906-16470e107-713001aa6de511e0-001-test/0
>>>>  -vnc :1 -boot dc -monitor stdio
>>> 
>>> It is used for the iscsi driver. You have a raw BDS (growable == false)
>>> on top of an iscsi one (growable == true).
>> 
>> Ok, but growable == true is wrong for the iSCSI driver isn`t it?
> 
> I guess it depends on your definition. If you say that growable includes
> the capability of growing the image, then yes, it's wrong. If you only
> interpret it as permission to write beyond EOF (if the driver supports
> that), then it's right even though any such attempt will result in an
> error.
> 
> Practically speaking, the difference is between -EIO returned from
> bdrv_check_byte_request() and -EIO from iscsi_aio_write16_cb(), i.e.
> the result is the same.

Yes, but there are many broken storage implementations outside which might react
freaky if there is a read/write beyond the LUN size or with negative offset.
The check_request would block such requests earlier protecting the 
infrastructure.
I have a case open with the vendor of a storage system where I am able to crash
the storage sending illegal requests. I personally would feel more comfortable
if illegal requests where just not possible.

Peter