public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
From: Max Gurtovoy <mgurtovoy@nvidia.com>
To: <israelr@nvidia.com>, <stefanha@redhat.com>,
	<virtualization@lists.linux.dev>, <mst@redhat.com>,
	<linux-block@vger.kernel.org>
Cc: <oren@nvidia.com>, <nitzanc@nvidia.com>, <dbenbasat@nvidia.com>,
	<smalin@nvidia.com>, <larora@nvidia.com>, <izach@nvidia.com>,
	<aaptel@nvidia.com>, <parav@nvidia.com>, <kvm@vger.kernel.org>,
	Max Gurtovoy <mgurtovoy@nvidia.com>
Subject: [PATCH 1/2] virtio_blk: add length check for device writable portion
Date: Tue, 25 Feb 2025 01:31:05 +0200	[thread overview]
Message-ID: <20250224233106.8519-2-mgurtovoy@nvidia.com> (raw)
In-Reply-To: <20250224233106.8519-1-mgurtovoy@nvidia.com>

Add a safety check to ensure that the length of data written by the
device is at least as large the expected length. If this condition is
not met, it indicates a potential error in the device's response.

This change aligns with the virtio specification, which states:
"The driver MUST NOT make assumptions about data in device-writable
buffers beyond the first len bytes, and SHOULD ignore this data."

By setting an error status when len is insufficient, we ensure that the
driver does not process potentially invalid or incomplete data from the
device.

Reviewed-by: Aurelien Aptel <aaptel@nvidia.com>
Signed-off-by: Lokesh Arora <larora@nvidia.com>
Signed-off-by: Israel Rukshin <israelr@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 drivers/block/virtio_blk.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6a61ec35f426..58407cfee3ee 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -331,6 +331,20 @@ static inline u8 virtblk_vbr_status(struct virtblk_req *vbr)
 	return *((u8 *)&vbr->in_hdr + vbr->in_hdr_len - 1);
 }
 
+static inline void virtblk_vbr_set_err_status_upon_len_err(struct virtblk_req *vbr,
+		struct request *req, unsigned int len)
+{
+	unsigned int expected_len = vbr->in_hdr_len;
+
+	if (rq_dma_dir(req) == DMA_FROM_DEVICE)
+		expected_len += blk_rq_payload_bytes(req);
+
+	if (unlikely(len < expected_len)) {
+		u8 *status_ptr = (u8 *)&vbr->in_hdr + vbr->in_hdr_len - 1;
+		*status_ptr = VIRTIO_BLK_S_IOERR;
+	}
+}
+
 static inline void virtblk_request_done(struct request *req)
 {
 	struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
@@ -362,6 +376,9 @@ static void virtblk_done(struct virtqueue *vq)
 		while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {
 			struct request *req = blk_mq_rq_from_pdu(vbr);
 
+			/* Check device writable portion length, and fail upon error */
+			virtblk_vbr_set_err_status_upon_len_err(vbr, req, len);
+
 			if (likely(!blk_should_fake_timeout(req->q)))
 				blk_mq_complete_request(req);
 			req_done = true;
@@ -1208,6 +1225,9 @@ static int virtblk_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
 	while ((vbr = virtqueue_get_buf(vq->vq, &len)) != NULL) {
 		struct request *req = blk_mq_rq_from_pdu(vbr);
 
+		/* Check device writable portion length, and fail upon error */
+		virtblk_vbr_set_err_status_upon_len_err(vbr, req, len);
+
 		found++;
 		if (!blk_mq_complete_request_remote(req) &&
 		    !blk_mq_add_to_batch(req, iob, virtblk_vbr_status(vbr),
-- 
2.18.1


  reply	other threads:[~2025-02-24 23:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-24 23:31 [PATCH v1 0/2] virtio: Add length checks for device writable portions Max Gurtovoy
2025-02-24 23:31 ` Max Gurtovoy [this message]
2025-05-14 12:41   ` [PATCH 1/2] virtio_blk: add length check for device writable portion Michael S. Tsirkin
2025-02-24 23:31 ` [PATCH 2/2] virtio_fs: " Max Gurtovoy
2025-02-27  8:17 ` [PATCH v1 0/2] virtio: Add length checks for device writable portions Stefan Hajnoczi
2025-02-27  8:53   ` Michael S. Tsirkin
2025-02-27 13:53     ` Max Gurtovoy
2025-05-14 12:40   ` Michael S. Tsirkin

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=20250224233106.8519-2-mgurtovoy@nvidia.com \
    --to=mgurtovoy@nvidia.com \
    --cc=aaptel@nvidia.com \
    --cc=dbenbasat@nvidia.com \
    --cc=israelr@nvidia.com \
    --cc=izach@nvidia.com \
    --cc=kvm@vger.kernel.org \
    --cc=larora@nvidia.com \
    --cc=linux-block@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=nitzanc@nvidia.com \
    --cc=oren@nvidia.com \
    --cc=parav@nvidia.com \
    --cc=smalin@nvidia.com \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux.dev \
    /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