qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org
Subject: [PULL 21/31] block/export: port virtio-blk discard/write zeroes input validation
Date: Fri,  5 Mar 2021 17:54:44 +0100	[thread overview]
Message-ID: <20210305165454.356840-22-kwolf@redhat.com> (raw)
In-Reply-To: <20210305165454.356840-1-kwolf@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>

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 <stefanha@redhat.com>
Message-Id: <20210223144653.811468-11-stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 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 f74796241c..04044228d4 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:
@@ -362,11 +430,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.29.2



  parent reply	other threads:[~2021-03-05 17:19 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-05 16:54 [PULL 00/31] Block layer patches Kevin Wolf
2021-03-05 16:54 ` [PULL 01/31] iotests: Drop deprecated 'props' from object-add Kevin Wolf
2021-03-05 16:54 ` [PULL 02/31] backup: Remove nodes from job in .clean() Kevin Wolf
2021-03-05 16:54 ` [PULL 03/31] backup-top: Refuse I/O in inactive state Kevin Wolf
2021-03-05 16:54 ` [PULL 04/31] iotests/283: Check that finalize drops backup-top Kevin Wolf
2021-03-05 16:54 ` [PULL 05/31] iotests: Fix up python style in 300 Kevin Wolf
2021-03-05 16:54 ` [PULL 06/31] blockjob: report a better error message Kevin Wolf
2021-03-05 16:54 ` [PULL 07/31] storage-daemon: report unexpected arguments on the fly Kevin Wolf
2021-03-05 16:54 ` [PULL 08/31] storage-daemon: include current command line option in the errors Kevin Wolf
2021-03-05 16:54 ` [PULL 09/31] qemu-storage-daemon: add --pidfile option Kevin Wolf
2021-03-05 16:54 ` [PULL 10/31] docs: show how to spawn qemu-storage-daemon with fd passing Kevin Wolf
2021-03-05 16:54 ` [PULL 11/31] docs: replace insecure /tmp examples in qsd docs Kevin Wolf
2021-03-05 16:54 ` [PULL 12/31] vhost-user-blk: fix blkcfg->num_queues endianness Kevin Wolf
2021-03-05 16:54 ` [PULL 13/31] libqtest: add qtest_socket_server() Kevin Wolf
2021-03-05 16:54 ` [PULL 14/31] libqtest: add qtest_kill_qemu() Kevin Wolf
2021-03-05 16:54 ` [PULL 15/31] libqtest: add qtest_remove_abrt_handler() Kevin Wolf
2021-03-05 16:54 ` [PULL 16/31] test: new qTest case to test the vhost-user-blk-server Kevin Wolf
2021-03-05 16:54 ` [PULL 17/31] tests/qtest: add multi-queue test case to vhost-user-blk-test Kevin Wolf
2021-03-05 16:54 ` [PULL 18/31] block/export: fix blk_size double byteswap Kevin Wolf
2021-03-05 16:54 ` [PULL 19/31] block/export: use VIRTIO_BLK_SECTOR_BITS Kevin Wolf
2021-03-05 16:54 ` [PULL 20/31] block/export: fix vhost-user-blk export sector number calculation Kevin Wolf
2021-03-05 16:54 ` Kevin Wolf [this message]
2021-03-05 16:54 ` [PULL 22/31] vhost-user-blk-test: test discard/write zeroes invalid inputs Kevin Wolf
2021-03-05 16:54 ` [PULL 23/31] block/export: port virtio-blk read/write range check Kevin Wolf
2021-03-05 16:54 ` [PULL 24/31] qcow2-bitmap: make bytes_covered_by_bitmap_cluster() public Kevin Wolf
2021-03-05 16:54 ` [PULL 25/31] parallels.txt: fix bitmap L1 table description Kevin Wolf
2021-03-05 16:54 ` [PULL 26/31] block/parallels: BDRVParallelsState: add cluster_size field Kevin Wolf
2021-03-05 16:54 ` [PULL 27/31] parallels: support bitmap extension for read-only mode Kevin Wolf
2021-03-05 16:54 ` [PULL 28/31] iotests.py: add unarchive_sample_image() helper Kevin Wolf
2021-03-05 16:54 ` [PULL 29/31] iotests: add parallels-read-bitmap test Kevin Wolf
2021-03-05 16:54 ` [PULL 30/31] MAINTAINERS: update parallels block driver Kevin Wolf
2021-03-05 16:54 ` [PULL 31/31] docs: qsd: Explain --export nbd,name=... default Kevin Wolf
2021-03-06 11:22 ` [PULL 00/31] Block layer patches Peter Maydell
2021-03-08 11:08   ` Kevin Wolf
2021-03-08 17:10     ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210305165454.356840-22-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).