From 400ddccbcd8ddc13c85dbb7796b15fe9d6a01c1f Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 15 Mar 2021 18:16:28 -0400 Subject: [PATCH 12/15] block/export: port virtio-blk discard/write zeroes input validation RH-Author: Stefan Hajnoczi Message-id: <20210315181629.212884-6-stefanha@redhat.com> Patchwork-id: 101342 O-Subject: [RHEL-AV-8.4.0 qemu-kvm PATCH 5/6] block/export: port virtio-blk discard/write zeroes input validation Bugzilla: 1937004 RH-Acked-by: Danilo de Paula RH-Acked-by: Kevin Wolf RH-Acked-by: Max Reitz Validate discard/write zeroes the same way we do for virtio-blk. Some of these checks are mandated by the VIRTIO specification, others are internal to QEMU. Signed-off-by: Stefan Hajnoczi Message-Id: <20210223144653.811468-11-stefanha@redhat.com> Signed-off-by: Kevin Wolf (cherry picked from commit db4eadf9f10e19f864d70d1df3a90fbda31b8c06) Signed-off-by: Stefan Hajnoczi Signed-off-by: Danilo C. L. de Paula --- block/export/vhost-user-blk-server.c | 116 +++++++++++++++++++++------ 1 file changed, 93 insertions(+), 23 deletions(-) diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c index bb07f499c8..937bb5e9b4 100644 --- a/block/export/vhost-user-blk-server.c +++ b/block/export/vhost-user-blk-server.c @@ -29,6 +29,8 @@ enum { VHOST_USER_BLK_NUM_QUEUES_DEFAULT = 1, + VHOST_USER_BLK_MAX_DISCARD_SECTORS = 32768, + VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS = 32768, }; struct virtio_blk_inhdr { unsigned char status; @@ -65,30 +67,102 @@ static void vu_blk_req_complete(VuBlkReq *req) free(req); } +static bool vu_blk_sect_range_ok(VuBlkExport *vexp, uint64_t sector, + size_t size) +{ + uint64_t nb_sectors = size >> BDRV_SECTOR_BITS; + uint64_t total_sectors; + + if (nb_sectors > BDRV_REQUEST_MAX_SECTORS) { + return false; + } + if ((sector << VIRTIO_BLK_SECTOR_BITS) % vexp->blk_size) { + return false; + } + blk_get_geometry(vexp->export.blk, &total_sectors); + if (sector > total_sectors || nb_sectors > total_sectors - sector) { + return false; + } + return true; +} + static int coroutine_fn -vu_blk_discard_write_zeroes(BlockBackend *blk, struct iovec *iov, +vu_blk_discard_write_zeroes(VuBlkExport *vexp, struct iovec *iov, uint32_t iovcnt, uint32_t type) { + BlockBackend *blk = vexp->export.blk; struct virtio_blk_discard_write_zeroes desc; - ssize_t size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc)); + ssize_t size; + uint64_t sector; + uint32_t num_sectors; + uint32_t max_sectors; + uint32_t flags; + int bytes; + + /* Only one desc is currently supported */ + if (unlikely(iov_size(iov, iovcnt) > sizeof(desc))) { + return VIRTIO_BLK_S_UNSUPP; + } + + size = iov_to_buf(iov, iovcnt, 0, &desc, sizeof(desc)); if (unlikely(size != sizeof(desc))) { - error_report("Invalid size %zd, expect %zu", size, sizeof(desc)); - return -EINVAL; + error_report("Invalid size %zd, expected %zu", size, sizeof(desc)); + return VIRTIO_BLK_S_IOERR; } - uint64_t range[2] = { le64_to_cpu(desc.sector) << 9, - le32_to_cpu(desc.num_sectors) << 9 }; - if (type == VIRTIO_BLK_T_DISCARD) { - if (blk_co_pdiscard(blk, range[0], range[1]) == 0) { - return 0; + sector = le64_to_cpu(desc.sector); + num_sectors = le32_to_cpu(desc.num_sectors); + flags = le32_to_cpu(desc.flags); + max_sectors = (type == VIRTIO_BLK_T_WRITE_ZEROES) ? + VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS : + VHOST_USER_BLK_MAX_DISCARD_SECTORS; + + /* This check ensures that 'bytes' fits in an int */ + if (unlikely(num_sectors > max_sectors)) { + return VIRTIO_BLK_S_IOERR; + } + + bytes = num_sectors << VIRTIO_BLK_SECTOR_BITS; + + if (unlikely(!vu_blk_sect_range_ok(vexp, sector, bytes))) { + return VIRTIO_BLK_S_IOERR; + } + + /* + * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard + * and write zeroes commands if any unknown flag is set. + */ + if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) { + return VIRTIO_BLK_S_UNSUPP; + } + + if (type == VIRTIO_BLK_T_WRITE_ZEROES) { + int blk_flags = 0; + + if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) { + blk_flags |= BDRV_REQ_MAY_UNMAP; + } + + if (blk_co_pwrite_zeroes(blk, sector << VIRTIO_BLK_SECTOR_BITS, + bytes, blk_flags) == 0) { + return VIRTIO_BLK_S_OK; } - } else if (type == VIRTIO_BLK_T_WRITE_ZEROES) { - if (blk_co_pwrite_zeroes(blk, range[0], range[1], 0) == 0) { - return 0; + } else if (type == VIRTIO_BLK_T_DISCARD) { + /* + * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for + * discard commands if the unmap flag is set. + */ + if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) { + return VIRTIO_BLK_S_UNSUPP; + } + + if (blk_co_pdiscard(blk, sector << VIRTIO_BLK_SECTOR_BITS, + bytes) == 0) { + return VIRTIO_BLK_S_OK; } } - return -EINVAL; + return VIRTIO_BLK_S_IOERR; } static void coroutine_fn vu_blk_virtio_process_req(void *opaque) @@ -177,19 +251,13 @@ static void coroutine_fn vu_blk_virtio_process_req(void *opaque) } case VIRTIO_BLK_T_DISCARD: case VIRTIO_BLK_T_WRITE_ZEROES: { - int rc; - if (!vexp->writable) { req->in->status = VIRTIO_BLK_S_IOERR; break; } - rc = vu_blk_discard_write_zeroes(blk, &elem->out_sg[1], out_num, type); - if (rc == 0) { - req->in->status = VIRTIO_BLK_S_OK; - } else { - req->in->status = VIRTIO_BLK_S_IOERR; - } + req->in->status = vu_blk_discard_write_zeroes(vexp, out_iov, out_num, + type); break; } default: @@ -360,11 +428,13 @@ vu_blk_initialize_config(BlockDriverState *bs, config->min_io_size = cpu_to_le16(1); config->opt_io_size = cpu_to_le32(1); config->num_queues = cpu_to_le16(num_queues); - config->max_discard_sectors = cpu_to_le32(32768); + config->max_discard_sectors = + cpu_to_le32(VHOST_USER_BLK_MAX_DISCARD_SECTORS); config->max_discard_seg = cpu_to_le32(1); config->discard_sector_alignment = cpu_to_le32(blk_size >> VIRTIO_BLK_SECTOR_BITS); - config->max_write_zeroes_sectors = cpu_to_le32(32768); + config->max_write_zeroes_sectors + = cpu_to_le32(VHOST_USER_BLK_MAX_WRITE_ZEROES_SECTORS); config->max_write_zeroes_seg = cpu_to_le32(1); } -- 2.27.0