Re: [PATCH v2 00/12] block/export: vhost-user-blk server tests and input validation

[Philippe Mathieu-Daudé]

On 2/19/21 11:38 PM, Peter Maydell wrote:
> On Mon, 15 Feb 2021 at 10:41, Kevin Wolf <kwolf@redhat.com> wrote:
>>
>> Am 07.12.2020 um 18:20 hat Stefan Hajnoczi geschrieben:
>>> v2:
>>>  * Add abrt handler that terminates qemu-storage-daemon to
>>>    vhost-user-blk-test. No more orphaned processes on test failure. [Peter]
>>>  * Fix sector number calculation in vhost-user-blk-server.c
>>>  * Introduce VIRTIO_BLK_SECTOR_BITS/SIZE to make code clearer [Max]
>>>  * Fix vhost-user-blk-server.c blk_size double byteswap
>>>  * Fix vhost-user-blk blkcfg->num_queues endianness [Peter]
>>>  * Squashed cleanups into Coiby vhost-user-blk-test commit so the code is
>>>    easier to review
>>>
>>> The vhost-user-blk server test was already in Michael Tsirkin's recent vhost
>>> pull request, but was dropped because it exposed vhost-user regressions
>>> (b7c1bd9d7848 and the Based-on tag below). Now that the vhost-user 
>>> regressions
>>> are fixed we can re-introduce the test case.
>>>
>>> This series adds missing input validation that led to a Coverity report. The
>>> virtio-blk read, write, discard, and write zeroes commands need to check
>>> sector/byte ranges and other inputs. This solves the issue Peter Maydell 
>>> raised
>>> in "[PATCH for-5.2] block/export/vhost-user-blk-server.c: Avoid potential
>>> integer overflow".
>>>
>>> Merging just the input validation patches would be possible too, but I 
>>> prefer
>>> to merge the corresponding tests so the code is exercised by the CI.
>>
>> Is this series still open? I don't see it in master.
> 
> The Coverity issue is still unfixed, at any rate...

Copying Coverity report here:

CID 1435956 Unintentional integer overflow

In vu_blk_discard_write_zeroes: An integer overflow occurs, with the
result converted to a wider integer type (CWE-190)

 61 static int coroutine_fn
 62 vu_blk_discard_write_zeroes(BlockBackend *blk, struct iovec *iov,
 63                             uint32_t iovcnt, uint32_t type)
 64 {
 65     struct virtio_blk_discard_write_zeroes desc;
 66     ssize_t size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc));
 67     if (unlikely(size != sizeof(desc))) {
 68         error_report("Invalid size %zd, expect %zu", size,
sizeof(desc));
 69         return -EINVAL;
 70     }
 71
 72     uint64_t range[2] = { le64_to_cpu(desc.sector) << 9,

CID 1435956 (#1 of 1): Unintentional integer overflow
(OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression
le32_to_cpu(desc.num_sectors) << 9 with type uint32_t (32 bits,
unsigned) is evaluated using 32-bit arithmetic, and then used in a
context that expects an expression of type uint64_t (64 bits, unsigned).

 73                           le32_to_cpu(desc.num_sectors) << 9 };