Re: [RFC PATCH] block/null: Use 'read-zeroes' mode by default

[Philippe Mathieu-Daudé]

On 2/9/21 6:11 PM, Eric Blake wrote:
> On 2/9/21 11:01 AM, Philippe Mathieu-Daudé wrote:
>> The null-co driver is meant for (performance) testing.
>> By default, read operation does nothing, the provided buffer
>> is not filled with zero values and its content is unchanged.
>>
>> This can confuse security experts. For example, using the default
>> null-co driver, buf[] is uninitialized, the blk_pread() call
>> succeeds and we then access uninitialized memory:
>>
>>   static int guess_disk_lchs(BlockBackend *blk,
>>                              int *pcylinders, int *pheads,
>>                              int *psectors)
>>   {
>>       uint8_t buf[BDRV_SECTOR_SIZE];
>>       ...
>>
>>       if (blk_pread(blk, 0, buf, BDRV_SECTOR_SIZE) < 0) {
>>           return -1;
>>       }
>>       /* test msdos magic */
>>       if (buf[510] != 0x55 || buf[511] != 0xaa) {
>>           return -1;
>>       }
>>
>> We could audit all the uninitialized buffers and the
>> bdrv_co_preadv() handlers, but it is simpler to change the
>> default of this testing driver. Performance tests will have
>> to adapt and use 'null-co,read-zeroes=on'.
> 
> Wouldn't this rather be read-zeroes=off when doing performance testing?

Oops, yes ;)

> 
>>
>> Suggested-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> RFC maybe a stricter approach is required?
> 
> Since the null driver is only for testing in the first place, opting in
> to speed over security seems like a reasonable tradeoff.  But I consider
> the patch incomplete without an audit of the iotests that will want to
> use explicit read-zeroes=off.

Correct. I don't know about each iotest but I can send a patch with
explicit option, so review would be trivial.

Thanks,

Phil.