* [PATCH v3 0/2] FBE virtualization: inline encryption for virtio-blk guests
@ 2026-09-20 12:24 Linlin Zhang
2026-09-20 12:24 ` [PATCH v3 1/2] virtio_blk: Add control virtqueue support Linlin Zhang
2026-09-20 12:24 ` [PATCH v3 2/2] virtio_blk: add inline encryption support Linlin Zhang
0 siblings, 2 replies; 7+ messages in thread
From: Linlin Zhang @ 2026-09-20 12:24 UTC (permalink / raw)
To: mst, jasowangio, axboe, ebiggers, stefanha
Cc: pbonzini, eperezma, xuanzhuo, virtualization, linux-block,
linux-kernel
From: linlzhan <linlin.zhang@oss.qualcomm.com>
The virtio-blk driver currently does not preserve blk-crypto metadata when
dispatching encrypted bios to the virtio queue. As a result, inline
encryption cannot be used for virtio block devices.
This series enables inline encryption for guest VMs on platforms where the
Inline Crypto Engine (ICE) is owned by the host or another virtual machine.
It extends virtio-blk with a crypto control virtqueue and carries the
required encryption metadata with data requests.
The series consists of:
1. Add control virtqueue support.
This allows the guest to exchange key management requests with the
virtio-blk backend without mixing them with regular I/O requests.
2. Add inline encryption support.
The driver advertises the device encryption capabilities through a
struct blk_crypto_profile and carries encryption metadata, including
the virtual keyslot and data unit number (DUN), in virtio requests.
On the backend (blk-crypto-proxy, will be committed in another patch as
suggested.), the key table mapping virtual slot to the block crypto
key is maintained, so that the request metadata can be used to resolve
the guest keyslot to the corresponding block crypto key and to reconstruct
the blk-crypto context before submitting the bio to the underlying block
device.
This keeps the guest integrated with the existing blk-crypto and filesystem
encryption frameworks while preserving inline-encryption semantics across
the virtualization boundary.
This is compatible for the virtio SPEC update which is under review:
https://lore.kernel.org/all/20260913161628.368484-1-linlin.zhang@oss.qualcomm.com/
Known limitations:
- Virtio block inline encryption depends on the new control virtqueue
- Inline encryption is mutually exclusive with VIRTIO_BLK_F_ZONED.
Testing:
Compilation pass on Linux-next.
End-to-end FBE virtualization with wrapped key enabled was validated
on top of gunyah hypervisor. wrapped_key_test is a local utility to
get wrapped key and ephemeral wrapped key via storage ioctl interfaces.
- /data/wrapped_key_test /dev/block/userdata generate
- /data/wrapped_key_test /dev/block/userdata prepare /data/lt_key.bin
- /data/fscryptctl insert_wrapped_key < /data/eph_key.bin
- /data/fscryptctl set_policy --identifier=20f553802e64e36b43469211266a5f1c /data/testing
- echo "data" > /data/testing/file.txt
- sync and reboot
- /data/wrapped_key_test /dev/block/userdata prepare /data/lt_key.bin
- /data/fscryptctl insert_wrapped_key < /data/eph_key_2.bin
- /data/fscryptctl set_policy --identifier=d8ca51d6d2094b73b2dae5ee7e3a10b6 /data/testing
- cat /data/testing/file.txt
---
Changes Changes v2 => v3:
- Fix issues reported by sashiko-bot
- Change to the pointer of struct completion in the control-queue
request to avoid DMA cacheline sharing
- Submit a control-queue request with a timeout monitor
- Freeze the data plane before marking the control queue as dead
- Transmit the kernel blk-crypto-mod-num and key_type to those
defined by virtio SPEC
- Replace GFP_KERNEL with GFP_NOIO when allocating the memory for
control-queue request in the key program/evict flow
- Move the check of inline encryption support to a independent
conditional branch.
v2: https://lore.kernel.org/all/20260914133733.15429-1-linlin.zhang@oss.qualcomm.com/
Changes v1 => v2:
- Use control virtqueue to perform key management requests
- Extend virtio_blk_crypto_msg::dun from a single __virtio64 to a
four-element __virtio64 array to support larger DUN sizes.
- Remove data_unit_size_bit in virtio_blk_crypto_msg struct
v1: https://lore.kernel.org/all/20260827160806.1295313-1-linlin.zhang@oss.qualcomm.com/
linlzhan (2):
virtio_blk: Add control virtqueue support
virtio_blk: add inline encryption support
drivers/block/Kconfig | 12 +
drivers/block/virtio_blk.c | 907 +++++++++++++++++++++++++++++++-
include/linux/virtio_blk.h | 87 +++
include/uapi/linux/virtio_blk.h | 124 ++++-
4 files changed, 1111 insertions(+), 19 deletions(-)
create mode 100644 include/linux/virtio_blk.h
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] virtio_blk: Add control virtqueue support
2026-09-20 12:24 [PATCH v3 0/2] FBE virtualization: inline encryption for virtio-blk guests Linlin Zhang
@ 2026-09-20 12:24 ` Linlin Zhang
2026-09-20 12:35 ` sashiko-bot
2026-09-20 12:24 ` [PATCH v3 2/2] virtio_blk: add inline encryption support Linlin Zhang
1 sibling, 1 reply; 7+ messages in thread
From: Linlin Zhang @ 2026-09-20 12:24 UTC (permalink / raw)
To: mst, jasowangio, axboe, ebiggers, stefanha
Cc: pbonzini, eperezma, xuanzhuo, virtualization, linux-block,
linux-kernel
From: linlzhan <linlin.zhang@oss.qualcomm.com>
Add support for the optional virtio-blk control virtqueue.
If control queue feature bit is negociated, this allows the driver
to manage control-queue requests independently from the data path
and to safely handle outstanding requests during device removal
and suspend.
No control command is submitted by this change. The control virtqueue
will be used by a subsequent inline encryption implementation.
Signed-off-by: linlzhan <linlin.zhang@oss.qualcomm.com>
---
drivers/block/virtio_blk.c | 241 +++++++++++++++++++++++++++++++-
include/uapi/linux/virtio_blk.h | 1 +
2 files changed, 238 insertions(+), 4 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 32bf3ba07a9d..2fad86e8f7a9 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -6,6 +6,7 @@
#include <linux/hdreg.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/completion.h>
#include <linux/interrupt.h>
#include <linux/virtio.h>
#include <linux/virtio_blk.h>
@@ -52,6 +53,15 @@ struct virtio_blk_vq {
char name[VQ_NAME_LEN];
} ____cacheline_aligned_in_smp;
+struct virtio_blk_ctrl_vq {
+ struct virtqueue *vq;
+ struct mutex mutex;
+ spinlock_t lock;
+ unsigned int inflight;
+ bool dead;
+ struct completion drained;
+};
+
struct virtio_blk {
/*
* This mutex must be held by anything that may run after
@@ -83,6 +93,9 @@ struct virtio_blk {
/* For zoned device */
unsigned int zone_sectors;
+
+ /* Control virtqueue state. */
+ struct virtio_blk_ctrl_vq ctrl_vq;
};
struct virtblk_req {
@@ -110,6 +123,20 @@ struct virtblk_req {
struct scatterlist sg[];
};
+struct virtblk_ctrl_request {
+ __virtio32 type;
+ u8 status;
+
+ struct completion *compl;
+ /*
+ * Set when virtblk_ctrl_vq_request()'s waiter timed out and moved on
+ * without freeing this request. Whichever of virtblk_ctrlq_callback()
+ * or virtblk_ctrl_vq_drain() later retrieves the buffer must free
+ * @compl and this struct instead of calling complete() on them.
+ */
+ bool abandoned;
+};
+
static inline blk_status_t virtblk_result(u8 status)
{
switch (status) {
@@ -863,11 +890,184 @@ static int virtblk_getgeo(struct gendisk *disk, struct hd_geometry *geo)
return ret;
}
+#define VIRTBLK_CTRL_VQ_TIMEOUT (10 * HZ)
+
+/* Prevent new submissions and wait for in-flight requests to complete. */
+static void virtblk_ctrl_vq_quiesce(struct virtio_blk *vblk)
+{
+ unsigned long flags;
+ bool need_wait;
+
+ if (!vblk->ctrl_vq.vq)
+ return;
+
+ init_completion(&vblk->ctrl_vq.drained);
+
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ vblk->ctrl_vq.dead = true;
+ need_wait = vblk->ctrl_vq.inflight != 0;
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+
+ if (need_wait &&
+ !wait_for_completion_timeout(&vblk->ctrl_vq.drained,
+ VIRTBLK_CTRL_VQ_TIMEOUT))
+ dev_warn(&vblk->vdev->dev,
+ "timed out waiting for control queue requests to complete\n");
+}
+
+/* Fail requests left in the control queue after reset. */
+static void virtblk_ctrl_vq_drain(struct virtio_blk *vblk)
+{
+ struct virtblk_ctrl_request *creq;
+ unsigned long flags;
+
+ if (!vblk->ctrl_vq.vq)
+ return;
+
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ while ((creq = virtqueue_detach_unused_buf(vblk->ctrl_vq.vq)) != NULL) {
+ bool abandoned = creq->abandoned;
+
+ if (WARN_ON_ONCE(!vblk->ctrl_vq.inflight))
+ ;
+ else
+ vblk->ctrl_vq.inflight--;
+ if (!abandoned)
+ creq->status = VIRTIO_BLK_S_IOERR;
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+ if (abandoned) {
+ kfree(creq->compl);
+ kfree(creq);
+ } else {
+ complete(creq->compl);
+ }
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ }
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+}
+
+static void virtblk_ctrlq_callback(struct virtqueue *vq)
+{
+ struct virtio_blk *vblk = vq->vdev->priv;
+ struct virtblk_ctrl_request *creq;
+ unsigned long flags;
+ unsigned int len;
+
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ do {
+ virtqueue_disable_cb(vq);
+ while ((creq = virtqueue_get_buf(vq, &len)) != NULL) {
+ bool drained = false;
+ bool abandoned = creq->abandoned;
+
+ if (WARN_ON_ONCE(!vblk->ctrl_vq.inflight)) {
+ /*
+ * Still resolve the request. Never leave a
+ * synchronous caller blocked because the accounting
+ * state was already inconsistent.
+ */
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+ if (abandoned) {
+ kfree(creq->compl);
+ kfree(creq);
+ } else {
+ complete(creq->compl);
+ }
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ continue;
+ }
+
+ if (--vblk->ctrl_vq.inflight == 0 && vblk->ctrl_vq.dead)
+ drained = true;
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+ if (drained)
+ complete(&vblk->ctrl_vq.drained);
+ if (abandoned) {
+ kfree(creq->compl);
+ kfree(creq);
+ } else {
+ complete(creq->compl);
+ }
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ }
+ } while (!virtqueue_enable_cb(vq));
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+}
+
+/* Submit a control-queue request and wait for completion. */
+static int virtblk_ctrl_vq_request(struct virtio_blk *vblk,
+ struct virtblk_ctrl_request *creq,
+ struct scatterlist *sgs[],
+ unsigned int out_sgs, unsigned int in_sgs)
+{
+ struct completion *comp;
+ unsigned long flags;
+ int err;
+
+ /*
+ * GFP_NOIO: this may be reached on the bio-submission path
+ * (memory reclaim writing back dirty pages to this same device),
+ * so GFP_KERNEL could self-deadlock.
+ */
+ comp = kmalloc_obj(*comp, GFP_NOIO);
+ if (!comp)
+ return -ENOMEM;
+ init_completion(comp);
+
+ mutex_lock(&vblk->ctrl_vq.mutex);
+ creq->compl = comp;
+ creq->abandoned = false;
+
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ if (vblk->ctrl_vq.dead) {
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+ mutex_unlock(&vblk->ctrl_vq.mutex);
+ kfree(comp);
+ return -ENODEV;
+ }
+ err = virtqueue_add_sgs(vblk->ctrl_vq.vq, sgs, out_sgs, in_sgs, creq, GFP_ATOMIC);
+ if (!err) {
+ vblk->ctrl_vq.inflight++;
+ virtqueue_kick(vblk->ctrl_vq.vq);
+ }
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+ if (err) {
+ mutex_unlock(&vblk->ctrl_vq.mutex);
+ kfree(comp);
+ return err;
+ }
+
+ if (wait_for_completion_timeout(comp, VIRTBLK_CTRL_VQ_TIMEOUT)) {
+ mutex_unlock(&vblk->ctrl_vq.mutex);
+ kfree(comp);
+ return 0;
+ }
+
+ /*
+ * The host hasn't responded within the timeout. @creq is still
+ * owned by the device, so don't touch its DMA-target fields or
+ * free it here. Mark it abandoned and hand ownership of both @creq
+ * and @comp to whichever of virtblk_ctrlq_callback() or
+ * virtblk_ctrl_vq_drain() retrieves the buffer later; unlock the
+ * mutex so subsequent requests aren't serialized behind an
+ * unresponsive host.
+ */
+ spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
+ creq->abandoned = true;
+ spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
+ mutex_unlock(&vblk->ctrl_vq.mutex);
+
+ dev_warn(&vblk->vdev->dev,
+ "control queue request timed out, abandoning\n");
+ return -ETIMEDOUT;
+}
+
static void virtblk_free_disk(struct gendisk *disk)
{
struct virtio_blk *vblk = disk->private_data;
ida_free(&vd_index_ida, vblk->index);
+ mutex_destroy(&vblk->ctrl_vq.mutex);
mutex_destroy(&vblk->vdev_mutex);
kfree(vblk);
}
@@ -965,6 +1165,8 @@ static int init_vq(struct virtio_blk *vblk)
struct virtqueue **vqs;
unsigned short num_vqs;
unsigned short num_poll_vqs;
+ unsigned short total_vqs;
+ bool has_ctrl_vq;
struct virtio_device *vdev = vblk->vdev;
struct irq_affinity desc = { 0, };
@@ -993,12 +1195,19 @@ static int init_vq(struct virtio_blk *vblk)
vblk->io_queues[HCTX_TYPE_READ],
vblk->io_queues[HCTX_TYPE_POLL]);
+ /*
+ * The control vq is appended after the data vqs whenever
+ * F_CTRL_VQ is negotiated.
+ */
+ has_ctrl_vq = virtio_has_feature(vdev, VIRTIO_BLK_F_CTRL_VQ);
+ total_vqs = num_vqs + (has_ctrl_vq ? 1 : 0);
+
vblk->vqs = kmalloc_objs(*vblk->vqs, num_vqs);
if (!vblk->vqs)
return -ENOMEM;
- vqs_info = kzalloc_objs(*vqs_info, num_vqs);
- vqs = kmalloc_objs(*vqs, num_vqs);
+ vqs_info = kzalloc_objs(*vqs_info, total_vqs);
+ vqs = kmalloc_objs(*vqs, total_vqs);
if (!vqs_info || !vqs) {
err = -ENOMEM;
goto out;
@@ -1015,8 +1224,13 @@ static int init_vq(struct virtio_blk *vblk)
vqs_info[i].name = vblk->vqs[i].name;
}
+ if (has_ctrl_vq) {
+ vqs_info[num_vqs].callback = virtblk_ctrlq_callback;
+ vqs_info[num_vqs].name = "control";
+ }
+
/* Discover virtqueues and write information to configuration. */
- err = virtio_find_vqs(vdev, num_vqs, vqs, vqs_info, &desc);
+ err = virtio_find_vqs(vdev, total_vqs, vqs, vqs_info, &desc);
if (err)
goto out;
@@ -1025,6 +1239,9 @@ static int init_vq(struct virtio_blk *vblk)
vblk->vqs[i].vq = vqs[i];
}
vblk->num_vqs = num_vqs;
+ vblk->ctrl_vq.vq = has_ctrl_vq ? vqs[num_vqs] : NULL;
+ vblk->ctrl_vq.dead = false;
+ vblk->ctrl_vq.inflight = 0;
out:
kfree(vqs);
@@ -1464,14 +1681,18 @@ static int virtblk_probe(struct virtio_device *vdev)
}
mutex_init(&vblk->vdev_mutex);
+ mutex_init(&vblk->ctrl_vq.mutex);
+ spin_lock_init(&vblk->ctrl_vq.lock);
vblk->vdev = vdev;
INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
err = init_vq(vblk);
- if (err)
+ if (err) {
+ dev_err(&vdev->dev, "init virt queue failed: err = %d\n", err);
goto out_free_vblk;
+ }
/* Default queue sizing is to fill the ring. */
if (!virtblk_queue_depth) {
@@ -1553,6 +1774,7 @@ static int virtblk_probe(struct virtio_device *vdev)
out_free_vq:
vdev->config->del_vqs(vdev);
kfree(vblk->vqs);
+ vblk->ctrl_vq.vq = NULL;
out_free_vblk:
kfree(vblk);
out_free_index:
@@ -1571,16 +1793,21 @@ static void virtblk_remove(struct virtio_device *vdev)
del_gendisk(vblk->disk);
blk_mq_free_tag_set(&vblk->tag_set);
+ virtblk_ctrl_vq_quiesce(vblk);
+
mutex_lock(&vblk->vdev_mutex);
/* Stop all the virtqueues. */
virtio_reset_device(vdev);
+ virtblk_ctrl_vq_drain(vblk);
/* Virtqueues are stopped, nothing can use vblk->vdev anymore. */
vblk->vdev = NULL;
vdev->config->del_vqs(vdev);
kfree(vblk->vqs);
+ vblk->vqs = NULL;
+ vblk->ctrl_vq.vq = NULL;
mutex_unlock(&vblk->vdev_mutex);
@@ -1593,13 +1820,17 @@ static int virtblk_freeze_priv(struct virtio_device *vdev)
struct request_queue *q = vblk->disk->queue;
unsigned int memflags;
+
/* Ensure no requests in virtqueues before deleting vqs. */
memflags = blk_mq_freeze_queue(q);
blk_mq_quiesce_queue_nowait(q);
blk_mq_unfreeze_queue(q, memflags);
+ virtblk_ctrl_vq_quiesce(vblk);
+
/* Ensure we don't receive any more interrupts */
virtio_reset_device(vdev);
+ virtblk_ctrl_vq_drain(vblk);
/* Make sure no work handler is accessing the device. */
flush_work(&vblk->config_work);
@@ -1612,6 +1843,7 @@ static int virtblk_freeze_priv(struct virtio_device *vdev)
* pointers safely.
*/
vblk->vqs = NULL;
+ vblk->ctrl_vq.vq = NULL;
return 0;
}
@@ -1672,6 +1904,7 @@ static unsigned int features[] = {
VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES,
VIRTIO_BLK_F_SECURE_ERASE, VIRTIO_BLK_F_ZONED,
+ VIRTIO_BLK_F_CTRL_VQ,
};
static struct virtio_driver virtio_blk = {
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 3744e4da1b2a..0a16972a1535 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -42,6 +42,7 @@
#define VIRTIO_BLK_F_WRITE_ZEROES 14 /* WRITE ZEROES is supported */
#define VIRTIO_BLK_F_SECURE_ERASE 16 /* Secure Erase is supported */
#define VIRTIO_BLK_F_ZONED 17 /* Zoned block device */
+#define VIRTIO_BLK_F_CTRL_VQ 22 /* Control queue */
/* Legacy feature bits */
#ifndef VIRTIO_BLK_NO_LEGACY
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] virtio_blk: add inline encryption support
2026-09-20 12:24 [PATCH v3 0/2] FBE virtualization: inline encryption for virtio-blk guests Linlin Zhang
2026-09-20 12:24 ` [PATCH v3 1/2] virtio_blk: Add control virtqueue support Linlin Zhang
@ 2026-09-20 12:24 ` Linlin Zhang
2026-09-20 12:38 ` sashiko-bot
1 sibling, 1 reply; 7+ messages in thread
From: Linlin Zhang @ 2026-09-20 12:24 UTC (permalink / raw)
To: mst, jasowangio, axboe, ebiggers, stefanha
Cc: pbonzini, eperezma, xuanzhuo, virtualization, linux-block,
linux-kernel
From: linlzhan <linlin.zhang@oss.qualcomm.com>
Add support for the virtio-blk inline encryption feature
(VIRTIO_BLK_F_INLINE_ENCRYPTION), which lets the guest offload
per-I/O encryption to the host's inline crypto engine instead of
doing it in software in the guest.
Advertise the device's inline encryption characteristics (key
types, supported crypto modes, max keyslots, DUN size) and wire
them up to a struct blk_crypto_profile so upper layers can attach
encryption contexts to bios as usual. Key management (program,
evict, generate, import, prepare key, derive software secret) is
carried out as control commands over the control virtqueue, and
per-request keyslot and data unit number are carried in an
extended request header for read/write commands.
Inline encryption is only enabled when both the control virtqueue
and inline encryption feature bits are negotiated, and is gated
behind a new VIRTIO_BLK_INLINE_ENCRYPTION Kconfig option that
depends on BLK_INLINE_ENCRYPTION, so kernels that don't select it
are unaffected.
Signed-off-by: linlzhan <linlin.zhang@oss.qualcomm.com>
---
drivers/block/Kconfig | 12 +
drivers/block/virtio_blk.c | 668 +++++++++++++++++++++++++++++++-
include/linux/virtio_blk.h | 87 +++++
include/uapi/linux/virtio_blk.h | 123 +++++-
4 files changed, 874 insertions(+), 16 deletions(-)
create mode 100644 include/linux/virtio_blk.h
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 858320b6ebb7..58bb050d4617 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -372,4 +372,16 @@ config BLK_DEV_ZONED_LOOP
If unsure, say N.
+config VIRTIO_BLK_INLINE_ENCRYPTION
+ tristate "Virtio block inline encryption support"
+ depends on VIRTIO_BLK && BLK_INLINE_ENCRYPTION
+ help
+ Say 'Y or M' here will allow the virtio block driver to route crypto
+ requests to a different operating system in a virtualized
+ environment. This is useful to encrypt the data stored in the storage
+ by using storage inline crypto engine. The control queue feature bit
+ must be negotiated to enable this functionality.
+
+ If unsure, say N.
+
endif # BLK_DEV
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 2fad86e8f7a9..30c303364ca9 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -17,6 +17,7 @@
#include <linux/numa.h>
#include <linux/vmalloc.h>
#include <uapi/linux/virtio_ring.h>
+#include <linux/blk-crypto-profile.h>
#define PART_BITS 4
#define VQ_NAME_LEN 16
@@ -94,13 +95,24 @@ struct virtio_blk {
/* For zoned device */
unsigned int zone_sectors;
+ /* For inline encryption support */
+ struct blk_crypto_profile profile;
+ bool crypto_profile_initialized;
+
/* Control virtqueue state. */
struct virtio_blk_ctrl_vq ctrl_vq;
};
struct virtblk_req {
/* Out header */
- struct virtio_blk_outhdr out_hdr;
+ union {
+ struct virtio_blk_outhdr base;
+ struct {
+ struct virtio_blk_outhdr base;
+ /* Crypto message (if VIRTIO_BLK_F_INLINE_ENCRYPTION) */
+ struct virtio_blk_crypto_msg msg;
+ } crypto_append;
+ } out_hdr;
/* In header */
union {
@@ -124,7 +136,21 @@ struct virtblk_req {
};
struct virtblk_ctrl_request {
+ /* Type byte, always its own out-sg for every command. */
__virtio32 type;
+ /* Out request, sent as a second, separate out-sg if any. */
+ union {
+ struct virtio_blk_crypto_key_desc key_desc;
+ struct virtio_blk_crypto_key_blob blob;
+ } out_req;
+
+ /* In response */
+ union {
+ struct virtio_blk_crypto_key_blob blob;
+ struct virtio_blk_crypto_sw_secret secret;
+ struct virtio_blk_crypto_modes modes;
+ } in_resp;
+ /* Status byte, always its own in-sg for every command. */
u8 status;
struct completion *compl;
@@ -167,12 +193,17 @@ static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr)
{
struct scatterlist out_hdr, in_hdr, *sgs[3];
unsigned int num_out = 0, num_in = 0;
+ size_t out_hdr_len = sizeof(vbr->out_hdr.base);
- sg_init_one(&out_hdr, &vbr->out_hdr, sizeof(vbr->out_hdr));
+ if (vbr->out_hdr.base.type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_CRYPTO_IN) ||
+ vbr->out_hdr.base.type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_CRYPTO_OUT))
+ out_hdr_len = sizeof(vbr->out_hdr.crypto_append);
+
+ sg_init_one(&out_hdr, &vbr->out_hdr, out_hdr_len);
sgs[num_out++] = &out_hdr;
if (vbr->sg_table.nents) {
- if (vbr->out_hdr.type & cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT))
+ if (vbr->out_hdr.base.type & cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_OUT))
sgs[num_out++] = vbr->sg_table.sgl;
else
sgs[num_out + num_in++] = vbr->sg_table.sgl;
@@ -262,6 +293,22 @@ static void virtblk_cleanup_cmd(struct request *req)
kfree(bvec_virt(&req->special_vec));
}
+#if IS_ENABLED(CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION)
+static bool is_crypto_request(struct request *req)
+{
+ struct request_queue *q = req->q;
+
+ return q->crypto_profile &&
+ req->crypt_ctx &&
+ req->crypt_keyslot;
+}
+#else
+static inline bool is_crypto_request(struct request *req)
+{
+ return false;
+}
+#endif
+
static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev,
struct request *req,
struct virtblk_req *vbr)
@@ -270,20 +317,27 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev,
bool unmap = false;
u32 type;
u64 sector = 0;
+ int i;
if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED) && op_is_zone_mgmt(req_op(req)))
return BLK_STS_NOTSUPP;
/* Set fields for all request types */
- vbr->out_hdr.ioprio = cpu_to_virtio32(vdev, req_get_ioprio(req));
+ vbr->out_hdr.base.ioprio = cpu_to_virtio32(vdev, req_get_ioprio(req));
switch (req_op(req)) {
case REQ_OP_READ:
- type = VIRTIO_BLK_T_IN;
+ if (is_crypto_request(req))
+ type = VIRTIO_BLK_T_CRYPTO_IN;
+ else
+ type = VIRTIO_BLK_T_IN;
sector = blk_rq_pos(req);
break;
case REQ_OP_WRITE:
- type = VIRTIO_BLK_T_OUT;
+ if (is_crypto_request(req))
+ type = VIRTIO_BLK_T_CRYPTO_OUT;
+ else
+ type = VIRTIO_BLK_T_OUT;
sector = blk_rq_pos(req);
break;
case REQ_OP_FLUSH:
@@ -336,8 +390,8 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev,
/* Set fields for non-REQ_OP_DRV_IN request types */
vbr->in_hdr_len = in_hdr_len;
- vbr->out_hdr.type = cpu_to_virtio32(vdev, type);
- vbr->out_hdr.sector = cpu_to_virtio64(vdev, sector);
+ vbr->out_hdr.base.type = cpu_to_virtio32(vdev, type);
+ vbr->out_hdr.base.sector = cpu_to_virtio64(vdev, sector);
if (type == VIRTIO_BLK_T_DISCARD || type == VIRTIO_BLK_T_WRITE_ZEROES ||
type == VIRTIO_BLK_T_SECURE_ERASE) {
@@ -345,6 +399,18 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev,
return BLK_STS_RESOURCE;
}
+ if (type == VIRTIO_BLK_T_CRYPTO_IN || type == VIRTIO_BLK_T_CRYPTO_OUT) {
+ memset(&vbr->out_hdr.crypto_append.msg, 0,
+ sizeof(vbr->out_hdr.crypto_append.msg));
+ vbr->out_hdr.crypto_append.msg.slot =
+ cpu_to_virtio32(vdev,
+ blk_crypto_keyslot_index(req->crypt_keyslot));
+ for (i = 0; i < ARRAY_SIZE(vbr->out_hdr.crypto_append.msg.dun); i++) {
+ vbr->out_hdr.crypto_append.msg.dun[i] =
+ cpu_to_virtio64(vdev, req->crypt_ctx->bc_dun[i]);
+ }
+ }
+
return 0;
}
@@ -595,8 +661,8 @@ static int virtblk_submit_zone_report(struct virtio_blk *vblk,
vbr = blk_mq_rq_to_pdu(req);
vbr->in_hdr_len = sizeof(vbr->in_hdr.status);
- vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_ZONE_REPORT);
- vbr->out_hdr.sector = cpu_to_virtio64(vblk->vdev, sector);
+ vbr->out_hdr.base.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_ZONE_REPORT);
+ vbr->out_hdr.base.sector = cpu_to_virtio64(vblk->vdev, sector);
err = blk_rq_map_kern(req, report_buf, report_len, GFP_KERNEL);
if (err)
@@ -844,8 +910,8 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str)
vbr = blk_mq_rq_to_pdu(req);
vbr->in_hdr_len = sizeof(vbr->in_hdr.status);
- vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_GET_ID);
- vbr->out_hdr.sector = 0;
+ vbr->out_hdr.base.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_GET_ID);
+ vbr->out_hdr.base.sector = 0;
err = blk_rq_map_kern(req, id_str, VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
if (err)
@@ -1062,11 +1128,558 @@ static int virtblk_ctrl_vq_request(struct virtio_blk *vblk,
return -ETIMEDOUT;
}
+#if IS_ENABLED(CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION)
+static int virtblk_get_crypto_modes(struct virtio_blk *vblk,
+ unsigned int *crypto_modes_supported)
+{
+ unsigned int nr_modes = VIRTIO_BLK_CRYPTO_MODE_MAX + 1;
+ struct scatterlist type_sg, resp_sg, status_sg, *sgs[3];
+ struct virtblk_ctrl_request *creq;
+ unsigned int i;
+ int err;
+
+ creq = kzalloc_obj(*creq, GFP_KERNEL);
+ if (!creq)
+ return -ENOMEM;
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_GET_CRYPTO_MODES);
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&resp_sg, &creq->in_resp.modes, sizeof(creq->in_resp.modes));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &resp_sg;
+ sgs[2] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 1, 2);
+ if (err == -ETIMEDOUT)
+ return err;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+ if (err)
+ goto out_free;
+
+ for (i = 1; i < nr_modes; i++) {
+ u32 mode_mask = virtio32_to_cpu(vblk->vdev,
+ creq->in_resp.modes.modes[i]);
+ enum blk_crypto_mode_num mode = virtio_mode_to_blk(i);
+
+ if (!mode_mask)
+ continue;
+ if (!mode) {
+ dev_warn(&vblk->vdev->dev,
+ "ignoring unknown crypto mode %u\n", i);
+ continue;
+ }
+ crypto_modes_supported[mode] = mode_mask;
+ }
+
+out_free:
+ kfree(creq);
+ return err;
+}
+
+static int set_virtblk_crypto_key_desc(struct virtio_device *vdev,
+ struct virtblk_ctrl_request *creq,
+ const struct blk_crypto_key *key,
+ unsigned int slot)
+{
+ struct virtio_blk_crypto_key_desc *desc = &creq->out_req.key_desc;
+ unsigned int vtype = blk_key_type_to_virtio(key->crypto_cfg.key_type);
+
+ if (sizeof(desc->bytes) < key->size)
+ return -EOVERFLOW;
+ if (!vtype)
+ return -EOPNOTSUPP;
+
+ memset(desc, 0, sizeof(*desc));
+ desc->slot = cpu_to_virtio32(vdev, slot);
+ memcpy(desc->bytes, key->bytes, key->size);
+ desc->key_size = cpu_to_virtio32(vdev, key->size);
+ desc->crypto_mode = cpu_to_virtio32(vdev,
+ blk_mode_to_virtio(key->crypto_cfg.crypto_mode));
+ desc->key_type = cpu_to_virtio32(vdev, vtype);
+ desc->data_unit_size_bits = cpu_to_virtio32(vdev, key->data_unit_size_bits);
+ desc->dun_bytes = cpu_to_virtio32(vdev, key->crypto_cfg.dun_bytes);
+
+ return 0;
+}
+
+static inline struct virtio_blk *virtblk_from_profile(struct blk_crypto_profile *profile)
+{
+ return container_of(profile, struct virtio_blk, profile);
+}
+
+static int virtblk_crypto_keyslot_program(struct blk_crypto_profile *profile,
+ const struct blk_crypto_key *key,
+ unsigned int slot)
+{
+ struct virtio_blk *vblk = virtblk_from_profile(profile);
+ struct scatterlist type_sg, out_req_sg, status_sg, *sgs[3];
+ struct virtblk_ctrl_request *creq;
+ int err;
+
+ mutex_lock(&vblk->vdev_mutex);
+ if (!vblk->vdev) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ /*
+ * GFP_NOIO: this callback runs on the bio-submission path, which
+ * memory reclaim can reach while writing back dirty pages to this
+ * same device; GFP_KERNEL here could recurse into that same reclaim
+ * and self-deadlock.
+ */
+ creq = kzalloc_obj(*creq, GFP_NOIO);
+ if (!creq) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_KEYSLOT_PROGRAM);
+
+ err = set_virtblk_crypto_key_desc(vblk->vdev, creq, key, slot);
+ if (err)
+ goto out_free;
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&out_req_sg, &creq->out_req.key_desc, sizeof(creq->out_req.key_desc));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &out_req_sg;
+ sgs[2] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 1);
+ if (err == -ETIMEDOUT)
+ goto out_unlock;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+out_free:
+ kfree(creq);
+out_unlock:
+ mutex_unlock(&vblk->vdev_mutex);
+ return err;
+}
+
+static int virtblk_crypto_keyslot_evict(struct blk_crypto_profile *profile,
+ const struct blk_crypto_key *key,
+ unsigned int slot)
+{
+ struct virtio_blk *vblk = virtblk_from_profile(profile);
+ struct scatterlist type_sg, out_req_sg, status_sg, *sgs[3];
+ struct virtblk_ctrl_request *creq;
+ int err;
+
+ mutex_lock(&vblk->vdev_mutex);
+ if (!vblk->vdev) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ creq = kzalloc_obj(*creq, GFP_NOIO);
+ if (!creq) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_KEYSLOT_EVICT);
+
+ err = set_virtblk_crypto_key_desc(vblk->vdev, creq, key, slot);
+ if (err)
+ goto out_free;
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&out_req_sg, &creq->out_req.key_desc, sizeof(creq->out_req.key_desc));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &out_req_sg;
+ sgs[2] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 1);
+ if (err == -ETIMEDOUT)
+ goto out_unlock;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+out_free:
+ kfree(creq);
+out_unlock:
+ mutex_unlock(&vblk->vdev_mutex);
+ return err;
+}
+
+static int virtblk_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
+ const u8 *eph_key, size_t eph_key_size,
+ u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
+{
+ struct virtio_blk *vblk = virtblk_from_profile(profile);
+ struct scatterlist type_sg, out_req_sg, resp_sg, status_sg, *sgs[4];
+ struct virtblk_ctrl_request *creq;
+ int err;
+
+ mutex_lock(&vblk->vdev_mutex);
+ if (!vblk->vdev) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ if (eph_key_size > VIRTIO_BLK_CRYPTO_MAX_KEY_SIZE
+ || sizeof(creq->in_resp.secret.secret) < BLK_CRYPTO_SW_SECRET_SIZE) {
+ err = -EOVERFLOW;
+ goto out_unlock;
+ }
+
+ creq = kzalloc_obj(*creq, GFP_KERNEL);
+ if (!creq) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_DERIVE_SW_SECRET);
+ memcpy(creq->out_req.blob.key, eph_key, eph_key_size);
+ creq->out_req.blob.key_size = cpu_to_virtio32(vblk->vdev, eph_key_size);
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&out_req_sg, &creq->out_req.blob, sizeof(creq->out_req.blob));
+ sg_init_one(&resp_sg, &creq->in_resp.secret, sizeof(creq->in_resp.secret));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &out_req_sg;
+ sgs[2] = &resp_sg;
+ sgs[3] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 2);
+ if (err == -ETIMEDOUT)
+ goto out_unlock;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+ if (err)
+ goto out_free;
+
+ memcpy(sw_secret, creq->in_resp.secret.secret, BLK_CRYPTO_SW_SECRET_SIZE);
+out_free:
+ kfree(creq);
+out_unlock:
+ mutex_unlock(&vblk->vdev_mutex);
+ return err;
+}
+
+static int virtblk_crypto_generate_key(struct blk_crypto_profile *profile,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct virtio_blk *vblk = virtblk_from_profile(profile);
+ struct scatterlist type_sg, resp_sg, status_sg, *sgs[3];
+ struct virtblk_ctrl_request *creq;
+ unsigned int key_size;
+ int err;
+
+ mutex_lock(&vblk->vdev_mutex);
+ if (!vblk->vdev) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ creq = kzalloc_obj(*creq, GFP_KERNEL);
+ if (!creq) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_GENERATE_KEY);
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&resp_sg, &creq->in_resp.blob, sizeof(creq->in_resp.blob));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &resp_sg;
+ sgs[2] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 1, 2);
+ if (err == -ETIMEDOUT)
+ goto out_unlock;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+ if (err)
+ goto out_free;
+
+ key_size = virtio32_to_cpu(vblk->vdev, creq->in_resp.blob.key_size);
+ if (!key_size ||
+ key_size > BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE) {
+ dev_err(&vblk->vdev->dev,
+ "backend returned oversized generated key: %u\n", key_size);
+ err = -EOVERFLOW;
+ goto out_free;
+ }
+ memcpy(lt_key, creq->in_resp.blob.key, key_size);
+ err = key_size;
+out_free:
+ kfree(creq);
+out_unlock:
+ mutex_unlock(&vblk->vdev_mutex);
+ return err;
+}
+
+static int virtblk_crypto_prepare_key(struct blk_crypto_profile *profile,
+ const u8 *lt_key, size_t lt_key_size,
+ u8 eph_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct virtio_blk *vblk = virtblk_from_profile(profile);
+ struct scatterlist type_sg, out_req_sg, resp_sg, status_sg, *sgs[4];
+ struct virtblk_ctrl_request *creq;
+ unsigned int key_size;
+ int err;
+
+ mutex_lock(&vblk->vdev_mutex);
+ if (!vblk->vdev) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ if (lt_key_size > VIRTIO_BLK_CRYPTO_MAX_KEY_SIZE) {
+ err = -EOVERFLOW;
+ goto out_unlock;
+ }
+
+ creq = kzalloc_obj(*creq, GFP_KERNEL);
+ if (!creq) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_PREPARE_KEY);
+ memcpy(creq->out_req.blob.key, lt_key, lt_key_size);
+ creq->out_req.blob.key_size = cpu_to_virtio32(vblk->vdev, lt_key_size);
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&out_req_sg, &creq->out_req.blob, sizeof(creq->out_req.blob));
+ sg_init_one(&resp_sg, &creq->in_resp.blob, sizeof(creq->in_resp.blob));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &out_req_sg;
+ sgs[2] = &resp_sg;
+ sgs[3] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 2);
+ if (err == -ETIMEDOUT)
+ goto out_unlock;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+ if (err)
+ goto out_free;
+
+ key_size = virtio32_to_cpu(vblk->vdev, creq->in_resp.blob.key_size);
+ if (!key_size ||
+ key_size > BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE) {
+ dev_err(&vblk->vdev->dev,
+ "backend returned oversized prepared key: %u\n", key_size);
+ err = -EOVERFLOW;
+ goto out_free;
+ }
+ memcpy(eph_key, creq->in_resp.blob.key, key_size);
+ err = key_size;
+out_free:
+ kfree(creq);
+out_unlock:
+ mutex_unlock(&vblk->vdev_mutex);
+ return err;
+}
+
+static int virtblk_crypto_import_key(struct blk_crypto_profile *profile,
+ const u8 *raw_key, size_t raw_key_size,
+ u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
+{
+ struct virtio_blk *vblk = virtblk_from_profile(profile);
+ struct scatterlist type_sg, out_req_sg, resp_sg, status_sg, *sgs[4];
+ struct virtblk_ctrl_request *creq;
+ unsigned int key_size;
+ int err;
+
+ mutex_lock(&vblk->vdev_mutex);
+ if (!vblk->vdev) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ if (raw_key_size > VIRTIO_BLK_CRYPTO_MAX_KEY_SIZE) {
+ err = -EOVERFLOW;
+ goto out_unlock;
+ }
+
+ creq = kzalloc_obj(*creq, GFP_KERNEL);
+ if (!creq) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+
+ creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_IMPORT_KEY);
+ memcpy(creq->out_req.blob.key, raw_key, raw_key_size);
+ creq->out_req.blob.key_size = cpu_to_virtio32(vblk->vdev, raw_key_size);
+
+ sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
+ sg_init_one(&out_req_sg, &creq->out_req.blob, sizeof(creq->out_req.blob));
+ sg_init_one(&resp_sg, &creq->in_resp.blob, sizeof(creq->in_resp.blob));
+ sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
+ sgs[0] = &type_sg;
+ sgs[1] = &out_req_sg;
+ sgs[2] = &resp_sg;
+ sgs[3] = &status_sg;
+
+ err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 2);
+ if (err == -ETIMEDOUT)
+ goto out_unlock;
+ if (err)
+ goto out_free;
+
+ err = blk_status_to_errno(virtblk_result(creq->status));
+ if (err)
+ goto out_free;
+
+ key_size = virtio32_to_cpu(vblk->vdev, creq->in_resp.blob.key_size);
+ if (!key_size ||
+ key_size > BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE) {
+ dev_err(&vblk->vdev->dev,
+ "backend returned oversized imported key: %u\n", key_size);
+ err = -EOVERFLOW;
+ goto out_free;
+ }
+ memcpy(lt_key, creq->in_resp.blob.key, key_size);
+ err = key_size;
+out_free:
+ kfree(creq);
+out_unlock:
+ mutex_unlock(&vblk->vdev_mutex);
+ return err;
+}
+
+static const struct blk_crypto_ll_ops virtblk_crypto_ops = {
+ .keyslot_program = virtblk_crypto_keyslot_program,
+ .keyslot_evict = virtblk_crypto_keyslot_evict,
+ .derive_sw_secret = virtblk_crypto_derive_sw_secret,
+ .generate_key = virtblk_crypto_generate_key,
+ .prepare_key = virtblk_crypto_prepare_key,
+ .import_key = virtblk_crypto_import_key,
+};
+
+static unsigned int get_supported_blk_key_types(u8 virtio_key_types)
+{
+ unsigned int supported = 0;
+
+ if (virtio_key_types & VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW)
+ supported |= virtio_key_type_to_blk(VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW);
+ if (virtio_key_types & VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED)
+ supported |= virtio_key_type_to_blk(VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED);
+
+ return supported;
+}
+
+static int virtblk_init_crypto(struct virtio_blk *vblk)
+{
+ struct virtio_device *vdev = vblk->vdev;
+ unsigned int crypto_modes_supported[BLK_ENCRYPTION_MODE_MAX] = { 0 };
+ unsigned int key_type_supported;
+ u16 max_slots = 0;
+ /* virtio_cread() requires the variable size to match the config field exactly */
+ u8 max_dun_bytes = 0, key_types = 0;
+ int err;
+
+ virtio_cread(vdev, struct virtio_blk_config,
+ enc_characteristics.max_slots, &max_slots);
+ virtio_cread(vdev, struct virtio_blk_config,
+ enc_characteristics.max_dun_bytes, &max_dun_bytes);
+ virtio_cread(vdev, struct virtio_blk_config,
+ enc_characteristics.key_types, &key_types);
+
+ dev_info_once(&vdev->dev,
+ "max_slots = %u, max_dun_bytes = %u, key_types = 0x%x\n",
+ max_slots, max_dun_bytes, key_types);
+
+ if (!max_slots)
+ return -EINVAL;
+
+ /*
+ * struct virtio_blk_crypto_msg.dun is a fixed array of four __virtio64
+ * values (32 bytes total), matching the size of
+ * blk_crypto_ctx::bc_dun[4]. Refuse to advertise more than that as
+ * supported, or blk-crypto could negotiate a larger dun_bytes with the
+ * filesystem and have the high-order bytes of req->crypt_ctx->bc_dun
+ * silently dropped in virtblk_setup_cmd().
+ */
+ if (max_dun_bytes > sizeof_field(struct virtio_blk_crypto_msg, dun))
+ return -EINVAL;
+
+ key_type_supported = get_supported_blk_key_types(key_types);
+ if (!key_type_supported)
+ return -EINVAL;
+
+ err = virtblk_get_crypto_modes(vblk, crypto_modes_supported);
+ if (err) {
+ dev_err(&vdev->dev, "get crypto modes failed: %d\n", err);
+ return err;
+ }
+
+ /*
+ * Use the plain (non-devm) initializer: vblk->profile is embedded in
+ * struct virtio_blk, whose lifetime is tied to the gendisk, not to
+ * &vdev->dev. Tying destruction to the vdev via devm would run the
+ * destroy callback after virtblk_remove() has already freed vblk.
+ * virtblk_free_disk() calls blk_crypto_profile_destroy() explicitly
+ * instead, guarded by crypto_profile_initialized below.
+ */
+ err = blk_crypto_profile_init(&vblk->profile, max_slots);
+ if (err) {
+ dev_err(&vdev->dev, "crypto profile initialization failed: %d\n", err);
+ return err;
+ }
+
+ vblk->profile.ll_ops = virtblk_crypto_ops;
+ vblk->profile.max_dun_bytes_supported = max_dun_bytes;
+ vblk->profile.key_types_supported = key_type_supported;
+ vblk->profile.dev = &vdev->dev;
+ memcpy(vblk->profile.modes_supported, crypto_modes_supported,
+ BLK_ENCRYPTION_MODE_MAX * sizeof(unsigned int));
+
+ vblk->crypto_profile_initialized = true;
+
+ dev_info(&vdev->dev, "inline crypto profile initialized\n");
+
+ return 0;
+}
+
+static void virtblk_destroy_crypto(struct virtio_blk *vblk)
+{
+ if (vblk->crypto_profile_initialized)
+ blk_crypto_profile_destroy(&vblk->profile);
+}
+#else
+
+static inline int virtblk_init_crypto(struct virtio_blk *vblk)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline void virtblk_destroy_crypto(struct virtio_blk *vblk)
+{
+}
+#endif /* CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION */
+
static void virtblk_free_disk(struct gendisk *disk)
{
struct virtio_blk *vblk = disk->private_data;
ida_free(&vd_index_ida, vblk->index);
+ virtblk_destroy_crypto(vblk);
mutex_destroy(&vblk->ctrl_vq.mutex);
mutex_destroy(&vblk->vdev_mutex);
kfree(vblk);
@@ -1661,6 +2274,7 @@ static int virtblk_probe(struct virtio_device *vdev)
};
int err, index;
unsigned int queue_depth;
+ bool zoned_disk = false;
if (!vdev->config->get) {
dev_err(&vdev->dev, "%s failure: config access disabled\n",
@@ -1684,6 +2298,7 @@ static int virtblk_probe(struct virtio_device *vdev)
mutex_init(&vblk->ctrl_vq.mutex);
spin_lock_init(&vblk->ctrl_vq.lock);
+ vblk->crypto_profile_initialized = false;
vblk->vdev = vdev;
INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
@@ -1759,6 +2374,28 @@ static int virtblk_probe(struct virtio_device *vdev)
err = blk_revalidate_disk_zones(vblk->disk);
if (err)
goto out_cleanup_disk;
+
+ zoned_disk = true;
+ }
+
+ if (IS_ENABLED(CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION) &&
+ virtio_has_feature(vdev, VIRTIO_BLK_F_INLINE_ENCRYPTION) &&
+ virtio_has_feature(vdev, VIRTIO_BLK_F_CTRL_VQ)) {
+ if (zoned_disk) {
+ dev_info(&vdev->dev,
+ "inline crypto not supported on zoned device\n");
+ } else {
+ err = virtblk_init_crypto(vblk);
+ if (!err) {
+ if (!blk_crypto_register(&vblk->profile, vblk->disk->queue))
+ dev_warn(&vdev->dev,
+ "failed to register inline crypto profile\n");
+ } else {
+ dev_warn(&vdev->dev,
+ "inline crypto init failed: %d, continuing without inline crypto support\n",
+ err);
+ }
+ }
}
err = device_add_disk(&vdev->dev, vblk->disk, virtblk_attr_groups);
@@ -1858,6 +2495,11 @@ static int virtblk_restore_priv(struct virtio_device *vdev)
return ret;
virtio_device_ready(vdev);
+
+ /* Reprogram the keys to keyslots. */
+ if (vblk->crypto_profile_initialized)
+ blk_crypto_reprogram_all_keys(&vblk->profile);
+
blk_mq_unquiesce_queue(vblk->disk->queue);
return 0;
@@ -1904,7 +2546,7 @@ static unsigned int features[] = {
VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES,
VIRTIO_BLK_F_SECURE_ERASE, VIRTIO_BLK_F_ZONED,
- VIRTIO_BLK_F_CTRL_VQ,
+ VIRTIO_BLK_F_CTRL_VQ, VIRTIO_BLK_F_INLINE_ENCRYPTION,
};
static struct virtio_driver virtio_blk = {
diff --git a/include/linux/virtio_blk.h b/include/linux/virtio_blk.h
new file mode 100644
index 000000000000..9f5aecad1907
--- /dev/null
+++ b/include/linux/virtio_blk.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_VIRTIO_BLK_H
+#define _LINUX_VIRTIO_BLK_H
+
+#include <linux/blk-crypto.h>
+#include <uapi/linux/virtio_blk.h>
+
+#if IS_ENABLED(CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION)
+/**
+ * virtio_mode_to_blk() - Convert a virtio_blk crypto mode number to a block mode
+ * @vmode: The virtio_blk crypto mode number (VIRTIO_BLK_CRYPTO_MODE_*).
+ *
+ * Return: The corresponding &enum blk_crypto_mode_num, or
+ * %BLK_ENCRYPTION_MODE_INVALID if @vmode is out of range.
+ */
+static inline enum blk_crypto_mode_num virtio_mode_to_blk(unsigned int vmode)
+{
+ /* Indexed by virtio_blk crypto mode number; unlisted entries are 0 (INVALID). */
+ static const enum blk_crypto_mode_num modes[__VIRTIO_BLK_CRYPTO_MODE_MAX] = {
+ [VIRTIO_BLK_CRYPTO_MODE_AES_256_XTS] = BLK_ENCRYPTION_MODE_AES_256_XTS,
+ };
+
+ if (vmode >= __VIRTIO_BLK_CRYPTO_MODE_MAX)
+ return BLK_ENCRYPTION_MODE_INVALID;
+ return modes[vmode];
+}
+
+/**
+ * blk_mode_to_virtio() - Convert a block crypto mode to a virtio_blk crypto mode number
+ * @bmode: The kernel &enum blk_crypto_mode_num.
+ *
+ * Return: The corresponding virtio_blk crypto mode number, or
+ * %VIRTIO_BLK_CRYPTO_MODE_INVALID if @bmode is out of range.
+ */
+static inline unsigned int blk_mode_to_virtio(enum blk_crypto_mode_num bmode)
+{
+ /* Indexed by blk_crypto_mode_num; unlisted entries are 0 (INVALID). */
+ static const unsigned int modes[BLK_ENCRYPTION_MODE_MAX] = {
+ [BLK_ENCRYPTION_MODE_AES_256_XTS] = VIRTIO_BLK_CRYPTO_MODE_AES_256_XTS,
+ };
+
+ if (bmode >= BLK_ENCRYPTION_MODE_MAX)
+ return VIRTIO_BLK_CRYPTO_MODE_INVALID;
+ return modes[bmode];
+}
+
+/**
+ * virtio_key_type_to_blk() - Convert a virtio_blk crypto key type to a block key type
+ * @vtype: The virtio_blk crypto key type (VIRTIO_BLK_CRYPTO_KEY_TYPE_*).
+ *
+ * Return: The corresponding &enum blk_crypto_key_type, or 0 if @vtype does
+ * not name a single supported key type.
+ */
+static inline enum blk_crypto_key_type virtio_key_type_to_blk(unsigned int vtype)
+{
+ switch (vtype) {
+ case VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW:
+ return BLK_CRYPTO_KEY_TYPE_RAW;
+ case VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED:
+ return BLK_CRYPTO_KEY_TYPE_HW_WRAPPED;
+ default:
+ return 0;
+ }
+}
+
+/**
+ * blk_key_type_to_virtio() - Convert a block key type to a virtio_blk crypto key type
+ * @btype: The kernel &enum blk_crypto_key_type.
+ *
+ * Return: The corresponding virtio_blk crypto key type
+ * (VIRTIO_BLK_CRYPTO_KEY_TYPE_*), or 0 if @btype does not name a
+ * single supported key type.
+ */
+static inline unsigned int blk_key_type_to_virtio(enum blk_crypto_key_type btype)
+{
+ switch (btype) {
+ case BLK_CRYPTO_KEY_TYPE_RAW:
+ return VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW;
+ case BLK_CRYPTO_KEY_TYPE_HW_WRAPPED:
+ return VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED;
+ default:
+ return 0;
+ }
+}
+#endif /* CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION */
+
+#endif /* _LINUX_VIRTIO_BLK_H */
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 0a16972a1535..fc57407ff2c7 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -1,5 +1,5 @@
-#ifndef _LINUX_VIRTIO_BLK_H
-#define _LINUX_VIRTIO_BLK_H
+#ifndef _UAPI_LINUX_VIRTIO_BLK_H
+#define _UAPI_LINUX_VIRTIO_BLK_H
/* This header is BSD licensed so anyone can use the definitions to implement
* compatible drivers/servers.
*
@@ -43,6 +43,7 @@
#define VIRTIO_BLK_F_SECURE_ERASE 16 /* Secure Erase is supported */
#define VIRTIO_BLK_F_ZONED 17 /* Zoned block device */
#define VIRTIO_BLK_F_CTRL_VQ 22 /* Control queue */
+#define VIRTIO_BLK_F_INLINE_ENCRYPTION 23 /* Inline encryption */
/* Legacy feature bits */
#ifndef VIRTIO_BLK_NO_LEGACY
@@ -149,6 +150,15 @@ struct virtio_blk_config {
__u8 model;
__u8 unused2[3];
} zoned;
+
+ /* Inline Encryption device characteristics (if VIRTIO_BLK_F_INLINE_ENCRYPTION) */
+ struct virtio_blk_enc_characteristics {
+ __virtio16 max_slots;
+ __u8 max_dun_bytes;
+ /* Bitmask of supported key types: VIRTIO_BLK_CRYPTO_KEY_TYPE_* */
+ __u8 key_types;
+ __virtio32 unused3;
+ } enc_characteristics;
} __attribute__((packed));
/*
@@ -207,6 +217,33 @@ struct virtio_blk_config {
/* Reset All zones command */
#define VIRTIO_BLK_T_ZONE_RESET_ALL 26
+/* Inline-encrypted write: crypto_msg set in outhdr */
+#define VIRTIO_BLK_T_CRYPTO_OUT 27
+
+/* Inline-encrypted read: crypto_msg set in outhdr */
+#define VIRTIO_BLK_T_CRYPTO_IN 28
+
+/* Get inline crypto modes */
+#define VIRTIO_BLK_T_GET_CRYPTO_MODES 29
+
+/* Program a key into the keyslot */
+#define VIRTIO_BLK_T_CRYPTO_KEYSLOT_PROGRAM 30
+
+/* Evict a key */
+#define VIRTIO_BLK_T_CRYPTO_KEYSLOT_EVICT 31
+
+/* Derive the software secret from a hardware-wrapped key */
+#define VIRTIO_BLK_T_CRYPTO_DERIVE_SW_SECRET 32
+
+/* Generate a new hardware-wrapped key */
+#define VIRTIO_BLK_T_CRYPTO_GENERATE_KEY 33
+
+/* Import a raw key as a hardware-wrapped key */
+#define VIRTIO_BLK_T_CRYPTO_IMPORT_KEY 34
+
+/* Convert a long-term wrapped key to its ephemerally-wrapped form */
+#define VIRTIO_BLK_T_CRYPTO_PREPARE_KEY 35
+
#ifndef VIRTIO_BLK_NO_LEGACY
/* Barrier before this op. */
#define VIRTIO_BLK_T_BARRIER 0x80000000
@@ -226,6 +263,86 @@ struct virtio_blk_outhdr {
__virtio64 sector;
};
+/*
+ * Crypto message descriptor, appended to the outhdr of a
+ * VIRTIO_BLK_T_CRYPTO_OUT or VIRTIO_BLK_T_CRYPTO_IN request.
+ */
+struct virtio_blk_crypto_msg {
+ /* virtual key slot index */
+ __virtio32 slot;
+ __u8 unused[4];
+ /* data unit number (DUN / IV) for this request */
+ __virtio64 dun[4];
+};
+
+/* Key type for VIRTIO_BLK_F_INLINE_ENCRYPTION. */
+enum virtio_blk_crypto_key_type {
+ VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW = 1,
+ VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED,
+};
+
+/*
+ * Inline crypto key descriptor. Request part for
+ * VIRTIO_BLK_T_CRYPTO_KEYSLOT_PROGRAM/KEYSLOT_EVICT.
+ */
+/* Must be >= BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE in include/linux/blk-crypto.h */
+#define VIRTIO_BLK_CRYPTO_MAX_KEY_SIZE 128
+
+struct virtio_blk_crypto_key_desc {
+ __virtio32 slot;
+ __u8 bytes[VIRTIO_BLK_CRYPTO_MAX_KEY_SIZE];
+ __virtio32 key_size;
+ __virtio32 crypto_mode;
+ __virtio32 key_type;
+ __virtio32 data_unit_size_bits;
+ __virtio32 dun_bytes;
+};
+
+/*
+ * A raw or hardware-wrapped key blob, used as the request and/or reply part
+ * of the VIRTIO_BLK_T_CRYPTO_GENERATE_KEY/IMPORT_KEY/PREPARE_KEY/
+ * DERIVE_SW_SECRET commands.
+ */
+struct virtio_blk_crypto_key_blob {
+ __virtio32 key_size;
+ __u8 key[VIRTIO_BLK_CRYPTO_MAX_KEY_SIZE];
+};
+
+/* Must match BLK_CRYPTO_SW_SECRET_SIZE in include/linux/blk-crypto.h */
+#define VIRTIO_BLK_CRYPTO_SW_SECRET_SIZE 32
+
+/* Reply to a VIRTIO_BLK_T_CRYPTO_DERIVE_SW_SECRET request. */
+struct virtio_blk_crypto_sw_secret {
+ __u8 secret[VIRTIO_BLK_CRYPTO_SW_SECRET_SIZE];
+};
+
+/*
+ * Crypto mode numbers used in VIRTIO_BLK_T_GET_CRYPTO_MODES replies, in
+ * struct virtio_blk_crypto_key_desc.crypto_mode and in indexing struct
+ * virtio_blk_crypto_modes.modes[] below. These numbers are assigned by
+ * the virtio spec and are stable: a number is never reused for a different
+ * crypto mode, and additional crypto modes are assigned new, higher numbers.
+ */
+enum {
+ VIRTIO_BLK_CRYPTO_MODE_INVALID,
+ VIRTIO_BLK_CRYPTO_MODE_AES_256_XTS,
+ __VIRTIO_BLK_CRYPTO_MODE_MAX, /* sentinel: always one past the last real mode */
+};
+
+/* Highest crypto mode number defined by this version of the header. */
+#define VIRTIO_BLK_CRYPTO_MODE_MAX (__VIRTIO_BLK_CRYPTO_MODE_MAX - 1)
+
+/* Reply to a VIRTIO_BLK_T_GET_CRYPTO_MODES request. */
+struct virtio_blk_crypto_modes {
+ /*
+ * modes[N], for crypto mode number N <= VIRTIO_BLK_CRYPTO_MODE_MAX, is
+ * a bitmask of the data unit sizes with which crypto mode N can be
+ * used: bit i is set if a data unit size of (1 << i) bytes is
+ * supported. modes[0] is reserved and always 0.
+ */
+ __virtio32 modes[__VIRTIO_BLK_CRYPTO_MODE_MAX];
+};
+
/*
* Supported zoned device models.
*/
@@ -325,4 +442,4 @@ struct virtio_scsi_inhdr {
#define VIRTIO_BLK_S_ZONE_OPEN_RESOURCE 5
#define VIRTIO_BLK_S_ZONE_ACTIVE_RESOURCE 6
-#endif /* _LINUX_VIRTIO_BLK_H */
+#endif /* _UAPI_LINUX_VIRTIO_BLK_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] virtio_blk: Add control virtqueue support
2026-09-20 12:24 ` [PATCH v3 1/2] virtio_blk: Add control virtqueue support Linlin Zhang
@ 2026-09-20 12:35 ` sashiko-bot
2026-10-08 11:10 ` Linlin Zhang
0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-09-20 12:35 UTC (permalink / raw)
To: Linlin Zhang; +Cc: Eugenio Perez, Michael S. Tsirkin, virtualization
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] block/virtio_blk: DMA coherency violation on creq->abandoned
- [Medium] block/virtio_blk: TOCTOU race in control request timeout leads to memory leak
--
commit e5e45f06d7dfecf78d290eb12655787cf1bee763
Author: linlzhan <linlin.zhang@oss.qualcomm.com>
virtio_blk: Add control virtqueue support
This patch adds support for the optional virtio-blk control virtqueue,
allowing the driver to manage control-queue requests independently from
the data path. The control virtqueue is intended for a subsequent inline
encryption implementation.
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 32bf3ba07a9dc..2fad86e8f7a9a 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
[ ... ]
> @@ -110,6 +123,20 @@ struct virtblk_req {
> struct scatterlist sg[];
> };
>
> +struct virtblk_ctrl_request {
> + __virtio32 type;
> + u8 status;
> +
> + struct completion *compl;
> + /*
> + * Set when virtblk_ctrl_vq_request()'s waiter timed out and moved on
> + * without freeing this request. Whichever of virtblk_ctrlq_callback()
> + * or virtblk_ctrl_vq_drain() later retrieves the buffer must free
> + * @compl and this struct instead of calling complete() on them.
> + */
> + bool abandoned;
> +};
[Severity: High]
Could this cause a DMA coherency violation? The struct bundles the
device-written DMA field (status) with a CPU-modified software flag
(abandoned) within the same cacheline.
On non-cache-coherent architectures, if the CPU writes to the abandoned
flag while the device is actively DMA-writing to the status field, it
could result in cacheline tearing. This might lead to either silent data
corruption of the DMA-mapped response or loss of the CPU write.
[ ... ]
> +static void virtblk_ctrlq_callback(struct virtqueue *vq)
> +{
> + struct virtio_blk *vblk = vq->vdev->priv;
> + struct virtblk_ctrl_request *creq;
> + unsigned long flags;
> + unsigned int len;
> +
> + spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
> + do {
> + virtqueue_disable_cb(vq);
> + while ((creq = virtqueue_get_buf(vq, &len)) != NULL) {
> + bool drained = false;
> + bool abandoned = creq->abandoned;
> +
> + if (WARN_ON_ONCE(!vblk->ctrl_vq.inflight)) {
[ ... ]
> + if (drained)
> + complete(&vblk->ctrl_vq.drained);
> + if (abandoned) {
> + kfree(creq->compl);
> + kfree(creq);
> + } else {
> + complete(creq->compl);
> + }
> + spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
> + }
> + } while (!virtqueue_enable_cb(vq));
> + spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
> +}
[Severity: Medium]
Is there a race condition here that could lead to a memory leak of both
the request and completion structs?
If a request times out exactly as the device completes it, the IRQ thread
might read creq->abandoned as false, unlock the spinlock, and get
preempted.
Meanwhile, virtblk_ctrl_vq_request() wakes up from the timeout, acquires
the lock, sets creq->abandoned to true, unlocks, and returns -ETIMEDOUT.
When the callback resumes, it checks its local abandoned variable (which
is still false) and calls complete(creq->compl) instead of freeing the
memory. Because the caller already discarded the pointer upon timeout,
both creq and compl would be permanently leaked.
[ ... ]
> +/* Submit a control-queue request and wait for completion. */
> +static int virtblk_ctrl_vq_request(struct virtio_blk *vblk,
[ ... ]
> + if (wait_for_completion_timeout(comp, VIRTBLK_CTRL_VQ_TIMEOUT)) {
> + mutex_unlock(&vblk->ctrl_vq.mutex);
> + kfree(comp);
> + return 0;
> + }
> +
> + /*
> + * The host hasn't responded within the timeout. @creq is still
> + * owned by the device, so don't touch its DMA-target fields or
> + * free it here. Mark it abandoned and hand ownership of both @creq
> + * and @comp to whichever of virtblk_ctrlq_callback() or
> + * virtblk_ctrl_vq_drain() retrieves the buffer later; unlock the
> + * mutex so subsequent requests aren't serialized behind an
> + * unresponsive host.
> + */
> + spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
> + creq->abandoned = true;
> + spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
> + mutex_unlock(&vblk->ctrl_vq.mutex);
> +
> + dev_warn(&vblk->vdev->dev,
> + "control queue request timed out, abandoning\n");
> + return -ETIMEDOUT;
> +}
[Severity: High]
This timeout path is where the CPU writes to the shared cacheline while
the DMA operation might still be in flight. Does this trigger the DMA
coherency violation mentioned above for struct virtblk_ctrl_request?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920122444.2549493-1-linlin.zhang@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] virtio_blk: add inline encryption support
2026-09-20 12:24 ` [PATCH v3 2/2] virtio_blk: add inline encryption support Linlin Zhang
@ 2026-09-20 12:38 ` sashiko-bot
2026-10-08 11:10 ` Linlin Zhang
0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-09-20 12:38 UTC (permalink / raw)
To: Linlin Zhang; +Cc: Eugenio Perez, Michael S. Tsirkin, virtualization
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] block: memory leak of sensitive cryptographic key material in virtio_blk
- [High] block: DMA cacheline sharing in virtblk_ctrl_request causes data corruption
- [High] block: deadlock in virtblk_crypto_generate_key() due to vdev_mutex
- [Medium] block: incorrect bitwise arithmetic in get_supported_blk_key_types() breaks negotiation
- [Medium] block: Kconfig VIRTIO_BLK_INLINE_ENCRYPTION tristate causes linker errors
--
commit 3dd149ac489e952ca2319a45325b2d578747f639
Author: linlzhan <linlin.zhang@oss.qualcomm.com>
virtio_blk: add inline encryption support
This commit adds support for the virtio-blk inline encryption feature,
allowing the guest to offload per-I/O encryption to the host. It wires up
device characteristics to a blk_crypto_profile and manages keys via the
control virtqueue.
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 858320b6ebb7e..58bb050d4617c 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -372,4 +372,16 @@ config BLK_DEV_ZONED_LOOP
>
> If unsure, say N.
>
> +config VIRTIO_BLK_INLINE_ENCRYPTION
> + tristate "Virtio block inline encryption support"
> + depends on VIRTIO_BLK && BLK_INLINE_ENCRYPTION
[Severity: Medium]
Should this option be a bool instead of a tristate?
If a user configures CONFIG_VIRTIO_BLK=y and CONFIG_BLK_INLINE_ENCRYPTION=m,
this dependency evaluates to 'm'. Because the inline encryption code is
integrated directly into virtio_blk.c using IS_ENABLED(), it gets compiled
as built-in alongside virtio_blk. It will then attempt to link against
blk_crypto_profile_init() and other block crypto symbols which are loadable
modules, causing unresolved symbol errors during linking.
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 2fad86e8f7a9a..30c303364ca97 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
[ ... ]
> @@ -124,7 +136,21 @@ struct virtblk_req {
> };
>
> struct virtblk_ctrl_request {
> + /* Type byte, always its own out-sg for every command. */
> __virtio32 type;
> + /* Out request, sent as a second, separate out-sg if any. */
> + union {
> + struct virtio_blk_crypto_key_desc key_desc;
> + struct virtio_blk_crypto_key_blob blob;
> + } out_req;
> +
> + /* In response */
> + union {
> + struct virtio_blk_crypto_key_blob blob;
> + struct virtio_blk_crypto_sw_secret secret;
> + struct virtio_blk_crypto_modes modes;
> + } in_resp;
> + /* Status byte, always its own in-sg for every command. */
> u8 status;
>
> struct completion *compl;
[Severity: High]
Could this struct layout cause cacheline corruption on architectures with
non-coherent DMA?
The virtblk_ctrl_request struct dynamically allocates fields mapped for
DMA_TO_DEVICE (type, out_req) and DMA_FROM_DEVICE (status, in_resp),
alongside CPU-modified fields (abandoned, compl). Because the struct is
not cacheline aligned internally, these fields share CPU cachelines. If the
CPU writes to the abandoned field while a DMA operation is occurring, or
before dma_unmap_sg() invalidates the cache for DMA_FROM_DEVICE, the cache
coherency conflict might overwrite or discard data.
[ ... ]
> +static int virtblk_crypto_keyslot_program(struct blk_crypto_profile *profile,
> + const struct blk_crypto_key *key,
> + unsigned int slot)
> +{
[ ... ]
> + creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_KEYSLOT_PROGRAM);
> +
> + err = set_virtblk_crypto_key_desc(vblk->vdev, creq, key, slot);
> + if (err)
> + goto out_free;
> +
> + sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
> + sg_init_one(&out_req_sg, &creq->out_req.key_desc, sizeof(creq->out_req.key_desc));
> + sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
> + sgs[0] = &type_sg;
> + sgs[1] = &out_req_sg;
> + sgs[2] = &status_sg;
> +
> + err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 1);
[Severity: Medium]
This is a pre-existing issue, but does virtblk_ctrl_vq_request() contain a
race condition when handling timeouts for these allocated control requests?
If wait_for_completion_timeout() times out, virtblk_ctrl_vq_request() sets
creq->abandoned = true to inform the callback to free the memory. However, if
the interrupt fires exactly after the timeout expires but before the lock is
acquired, virtblk_ctrlq_callback() reads abandoned == false and assumes the
waiting thread will free it. The waiting thread then returns -ETIMEDOUT and
skips freeing it, leading to a permanent leak of both creq and compl.
> + if (err == -ETIMEDOUT)
> + goto out_unlock;
> + if (err)
> + goto out_free;
> +
> + err = blk_status_to_errno(virtblk_result(creq->status));
> +out_free:
> + kfree(creq);
[Severity: High]
Does this leak sensitive cryptographic key material in memory?
The creq buffer contains the plaintext or wrapped key descriptor, but it is
freed via kfree() instead of kfree_sensitive(). The fscrypt subsystem rules
mandate that keys must be zeroized after use to prevent memory disclosure.
[ ... ]
> +static int virtblk_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
> + const u8 *eph_key, size_t eph_key_size,
> + u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
> +{
[ ... ]
> + err = blk_status_to_errno(virtblk_result(creq->status));
> + if (err)
> + goto out_free;
> +
> + memcpy(sw_secret, creq->in_resp.secret.secret, BLK_CRYPTO_SW_SECRET_SIZE);
> +out_free:
> + kfree(creq);
[Severity: High]
Similar to virtblk_crypto_keyslot_program(), does this leave the highly
sensitive derived software secret in memory instead of zeroizing it with
kfree_sensitive()?
[ ... ]
> +static int virtblk_crypto_generate_key(struct blk_crypto_profile *profile,
> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
> +{
> + struct virtio_blk *vblk = virtblk_from_profile(profile);
> + struct scatterlist type_sg, resp_sg, status_sg, *sgs[3];
> + struct virtblk_ctrl_request *creq;
> + unsigned int key_size;
> + int err;
> +
> + mutex_lock(&vblk->vdev_mutex);
> + if (!vblk->vdev) {
> + err = -ENXIO;
> + goto out_unlock;
> + }
> +
> + creq = kzalloc_obj(*creq, GFP_KERNEL);
[Severity: High]
Can holding vdev_mutex during a GFP_KERNEL allocation cause a deadlock here?
Under memory pressure, GFP_KERNEL triggers direct reclaim, which flushes
dirty pages to the block device. If the bio requires inline encryption, the
block layer synchronously calls virtblk_crypto_keyslot_program() from the I/O
submission path. That callback immediately attempts to acquire vdev_mutex,
which is already held by the sleeping thread, causing a strict AB-BA/AA
deadlock.
[ ... ]
> +static unsigned int get_supported_blk_key_types(u8 virtio_key_types)
> +{
> + unsigned int supported = 0;
> +
> + if (virtio_key_types & VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW)
> + supported |= virtio_key_type_to_blk(VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW);
> + if (virtio_key_types & VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED)
> + supported |= virtio_key_type_to_blk(VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED);
> +
> + return supported;
> +}
[Severity: Medium]
Does this construct an invalid capability bitmask by directly bitwise ORing
raw enum values instead of shifting them?
The block layer's profile->key_types_supported strictly requires a bitmask
where support is represented as (1 << BLK_CRYPTO_KEY_TYPE_...). For
BLK_CRYPTO_KEY_TYPE_HW_WRAPPED (value 2), the accumulated enum value sets
bit 1, making supported equal to 3. The core block layer later checks
3 & (1 << 2), which evaluates to 0, incorrectly causing
blk_crypto_config_supported() to reject hardware-wrapped keys as unsupported.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920122444.2549493-1-linlin.zhang@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] virtio_blk: Add control virtqueue support
2026-09-20 12:35 ` sashiko-bot
@ 2026-10-08 11:10 ` Linlin Zhang
0 siblings, 0 replies; 7+ messages in thread
From: Linlin Zhang @ 2026-10-08 11:10 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Eugenio Perez, Michael S. Tsirkin, virtualization
On 9/20/2026 8:35 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] block/virtio_blk: DMA coherency violation on creq->abandoned
> - [Medium] block/virtio_blk: TOCTOU race in control request timeout leads to memory leak
> --
>
> commit e5e45f06d7dfecf78d290eb12655787cf1bee763
> Author: linlzhan <linlin.zhang@oss.qualcomm.com>
>
> virtio_blk: Add control virtqueue support
>
> This patch adds support for the optional virtio-blk control virtqueue,
> allowing the driver to manage control-queue requests independently from
> the data path. The control virtqueue is intended for a subsequent inline
> encryption implementation.
>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index 32bf3ba07a9dc..2fad86e8f7a9a 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
> [ ... ]
>> @@ -110,6 +123,20 @@ struct virtblk_req {
>> struct scatterlist sg[];
>> };
>>
>> +struct virtblk_ctrl_request {
>> + __virtio32 type;
>> + u8 status;
>> +
>> + struct completion *compl;
>> + /*
>> + * Set when virtblk_ctrl_vq_request()'s waiter timed out and moved on
>> + * without freeing this request. Whichever of virtblk_ctrlq_callback()
>> + * or virtblk_ctrl_vq_drain() later retrieves the buffer must free
>> + * @compl and this struct instead of calling complete() on them.
>> + */
>> + bool abandoned;
>> +};
>
> [Severity: High]
> Could this cause a DMA coherency violation? The struct bundles the
> device-written DMA field (status) with a CPU-modified software flag
> (abandoned) within the same cacheline.
>
> On non-cache-coherent architectures, if the CPU writes to the abandoned
> flag while the device is actively DMA-writing to the status field, it
> could result in cacheline tearing. This might lead to either silent data
> corruption of the DMA-mapped response or loss of the CPU write.
ACK
Fix it by moving both struct completion and bool abandoned to a pointer.
>
> [ ... ]
>> +static void virtblk_ctrlq_callback(struct virtqueue *vq)
>> +{
>> + struct virtio_blk *vblk = vq->vdev->priv;
>> + struct virtblk_ctrl_request *creq;
>> + unsigned long flags;
>> + unsigned int len;
>> +
>> + spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
>> + do {
>> + virtqueue_disable_cb(vq);
>> + while ((creq = virtqueue_get_buf(vq, &len)) != NULL) {
>> + bool drained = false;
>> + bool abandoned = creq->abandoned;
>> +
>> + if (WARN_ON_ONCE(!vblk->ctrl_vq.inflight)) {
> [ ... ]
>> + if (drained)
>> + complete(&vblk->ctrl_vq.drained);
>> + if (abandoned) {
>> + kfree(creq->compl);
>> + kfree(creq);
>> + } else {
>> + complete(creq->compl);
>> + }
>> + spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
>> + }
>> + } while (!virtqueue_enable_cb(vq));
>> + spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
>> +}
>
> [Severity: Medium]
> Is there a race condition here that could lead to a memory leak of both
> the request and completion structs?
>
> If a request times out exactly as the device completes it, the IRQ thread
> might read creq->abandoned as false, unlock the spinlock, and get
> preempted.
>
> Meanwhile, virtblk_ctrl_vq_request() wakes up from the timeout, acquires
> the lock, sets creq->abandoned to true, unlocks, and returns -ETIMEDOUT.
>
> When the callback resumes, it checks its local abandoned variable (which
> is still false) and calls complete(creq->compl) instead of freeing the
> memory. Because the caller already discarded the pointer upon timeout,
> both creq and compl would be permanently leaked.
ACK
Lock the whole code snippet of do-while statement.
>
> [ ... ]
>> +/* Submit a control-queue request and wait for completion. */
>> +static int virtblk_ctrl_vq_request(struct virtio_blk *vblk,
> [ ... ]
>> + if (wait_for_completion_timeout(comp, VIRTBLK_CTRL_VQ_TIMEOUT)) {
>> + mutex_unlock(&vblk->ctrl_vq.mutex);
>> + kfree(comp);
>> + return 0;
>> + }
>> +
>> + /*
>> + * The host hasn't responded within the timeout. @creq is still
>> + * owned by the device, so don't touch its DMA-target fields or
>> + * free it here. Mark it abandoned and hand ownership of both @creq
>> + * and @comp to whichever of virtblk_ctrlq_callback() or
>> + * virtblk_ctrl_vq_drain() retrieves the buffer later; unlock the
>> + * mutex so subsequent requests aren't serialized behind an
>> + * unresponsive host.
>> + */
>> + spin_lock_irqsave(&vblk->ctrl_vq.lock, flags);
>> + creq->abandoned = true;
>> + spin_unlock_irqrestore(&vblk->ctrl_vq.lock, flags);
>> + mutex_unlock(&vblk->ctrl_vq.mutex);
>> +
>> + dev_warn(&vblk->vdev->dev,
>> + "control queue request timed out, abandoning\n");
>> + return -ETIMEDOUT;
>> +}
>
> [Severity: High]
> This timeout path is where the CPU writes to the shared cacheline while
> the DMA operation might still be in flight. Does this trigger the DMA
> coherency violation mentioned above for struct virtblk_ctrl_request?
>
ACK
Move struct completion and bool abandoned in struct virtblk_ctrl_request
to virtblk_ctrl_completion *. one time kmalloc_obj() of
virtblk_ctrl_completion * allow them not in the scatterlist.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] virtio_blk: add inline encryption support
2026-09-20 12:38 ` sashiko-bot
@ 2026-10-08 11:10 ` Linlin Zhang
0 siblings, 0 replies; 7+ messages in thread
From: Linlin Zhang @ 2026-10-08 11:10 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Eugenio Perez, Michael S. Tsirkin, virtualization
On 9/20/2026 8:38 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] block: memory leak of sensitive cryptographic key material in virtio_blk
> - [High] block: DMA cacheline sharing in virtblk_ctrl_request causes data corruption
> - [High] block: deadlock in virtblk_crypto_generate_key() due to vdev_mutex
> - [Medium] block: incorrect bitwise arithmetic in get_supported_blk_key_types() breaks negotiation
> - [Medium] block: Kconfig VIRTIO_BLK_INLINE_ENCRYPTION tristate causes linker errors
> --
>
> commit 3dd149ac489e952ca2319a45325b2d578747f639
> Author: linlzhan <linlin.zhang@oss.qualcomm.com>
>
> virtio_blk: add inline encryption support
>
> This commit adds support for the virtio-blk inline encryption feature,
> allowing the guest to offload per-I/O encryption to the host. It wires up
> device characteristics to a blk_crypto_profile and manages keys via the
> control virtqueue.
>
>> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
>> index 858320b6ebb7e..58bb050d4617c 100644
>> --- a/drivers/block/Kconfig
>> +++ b/drivers/block/Kconfig
>> @@ -372,4 +372,16 @@ config BLK_DEV_ZONED_LOOP
>>
>> If unsure, say N.
>>
>> +config VIRTIO_BLK_INLINE_ENCRYPTION
>> + tristate "Virtio block inline encryption support"
>> + depends on VIRTIO_BLK && BLK_INLINE_ENCRYPTION
>
> [Severity: Medium]
> Should this option be a bool instead of a tristate?
>
> If a user configures CONFIG_VIRTIO_BLK=y and CONFIG_BLK_INLINE_ENCRYPTION=m,
> this dependency evaluates to 'm'. Because the inline encryption code is
> integrated directly into virtio_blk.c using IS_ENABLED(), it gets compiled
> as built-in alongside virtio_blk. It will then attempt to link against
> blk_crypto_profile_init() and other block crypto symbols which are loadable
> modules, causing unresolved symbol errors during linking.
No. it should be 'tristate', so because it's code is totally in virtio block driver.
keeping 'tristate' make it possible for virtio block driver in module form can
support inline encryption as well - both CONFIG_VIRTIO_BLK_INLINE_ENCRYPTION and
CONFIG_VIRTIO_BLK set as 'm'.
>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index 2fad86e8f7a9a..30c303364ca97 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
> [ ... ]
>> @@ -124,7 +136,21 @@ struct virtblk_req {
>> };
>>
>> struct virtblk_ctrl_request {
>> + /* Type byte, always its own out-sg for every command. */
>> __virtio32 type;
>> + /* Out request, sent as a second, separate out-sg if any. */
>> + union {
>> + struct virtio_blk_crypto_key_desc key_desc;
>> + struct virtio_blk_crypto_key_blob blob;
>> + } out_req;
>> +
>> + /* In response */
>> + union {
>> + struct virtio_blk_crypto_key_blob blob;
>> + struct virtio_blk_crypto_sw_secret secret;
>> + struct virtio_blk_crypto_modes modes;
>> + } in_resp;
>> + /* Status byte, always its own in-sg for every command. */
>> u8 status;
>>
>> struct completion *compl;
>
> [Severity: High]
> Could this struct layout cause cacheline corruption on architectures with
> non-coherent DMA?
>
> The virtblk_ctrl_request struct dynamically allocates fields mapped for
> DMA_TO_DEVICE (type, out_req) and DMA_FROM_DEVICE (status, in_resp),
> alongside CPU-modified fields (abandoned, compl). Because the struct is
> not cacheline aligned internally, these fields share CPU cachelines. If the
> CPU writes to the abandoned field while a DMA operation is occurring, or
> before dma_unmap_sg() invalidates the cache for DMA_FROM_DEVICE, the cache
> coherency conflict might overwrite or discard data.
ACK
Fix it by moving both struct completion and bool abandoned to a pointer.
>
> [ ... ]
>> +static int virtblk_crypto_keyslot_program(struct blk_crypto_profile *profile,
>> + const struct blk_crypto_key *key,
>> + unsigned int slot)
>> +{
> [ ... ]
>> + creq->type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_CRYPTO_KEYSLOT_PROGRAM);
>> +
>> + err = set_virtblk_crypto_key_desc(vblk->vdev, creq, key, slot);
>> + if (err)
>> + goto out_free;
>> +
>> + sg_init_one(&type_sg, &creq->type, sizeof(creq->type));
>> + sg_init_one(&out_req_sg, &creq->out_req.key_desc, sizeof(creq->out_req.key_desc));
>> + sg_init_one(&status_sg, &creq->status, sizeof(creq->status));
>> + sgs[0] = &type_sg;
>> + sgs[1] = &out_req_sg;
>> + sgs[2] = &status_sg;
>> +
>> + err = virtblk_ctrl_vq_request(vblk, creq, sgs, 2, 1);
>
> [Severity: Medium]
> This is a pre-existing issue, but does virtblk_ctrl_vq_request() contain a
> race condition when handling timeouts for these allocated control requests?
>
> If wait_for_completion_timeout() times out, virtblk_ctrl_vq_request() sets
> creq->abandoned = true to inform the callback to free the memory. However, if
> the interrupt fires exactly after the timeout expires but before the lock is
> acquired, virtblk_ctrlq_callback() reads abandoned == false and assumes the
> waiting thread will free it. The waiting thread then returns -ETIMEDOUT and
> skips freeing it, leading to a permanent leak of both creq and compl.
ACK
Move struct completion and bool abandoned in struct virtblk_ctrl_request
to virtblk_ctrl_completion *. one time kmalloc_obj() of
virtblk_ctrl_completion * allow them not in the scatterlist.
>
>> + if (err == -ETIMEDOUT)
>> + goto out_unlock;
>> + if (err)
>> + goto out_free;
>> +
>> + err = blk_status_to_errno(virtblk_result(creq->status));
>> +out_free:
>> + kfree(creq);
>
> [Severity: High]
> Does this leak sensitive cryptographic key material in memory?
>
> The creq buffer contains the plaintext or wrapped key descriptor, but it is
> freed via kfree() instead of kfree_sensitive(). The fscrypt subsystem rules
> mandate that keys must be zeroized after use to prevent memory disclosure.
>
> [ ... ]
>> +static int virtblk_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
>> + const u8 *eph_key, size_t eph_key_size,
>> + u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
>> +{
> [ ... ]
>> + err = blk_status_to_errno(virtblk_result(creq->status));
>> + if (err)
>> + goto out_free;
>> +
>> + memcpy(sw_secret, creq->in_resp.secret.secret, BLK_CRYPTO_SW_SECRET_SIZE);
>> +out_free:
>> + kfree(creq);
>
> [Severity: High]
> Similar to virtblk_crypto_keyslot_program(), does this leave the highly
> sensitive derived software secret in memory instead of zeroizing it with
> kfree_sensitive()?
>
> [ ... ]
>> +static int virtblk_crypto_generate_key(struct blk_crypto_profile *profile,
>> + u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE])
>> +{
>> + struct virtio_blk *vblk = virtblk_from_profile(profile);
>> + struct scatterlist type_sg, resp_sg, status_sg, *sgs[3];
>> + struct virtblk_ctrl_request *creq;
>> + unsigned int key_size;
>> + int err;
>> +
>> + mutex_lock(&vblk->vdev_mutex);
>> + if (!vblk->vdev) {
>> + err = -ENXIO;
>> + goto out_unlock;
>> + }
>> +
>> + creq = kzalloc_obj(*creq, GFP_KERNEL);
>
> [Severity: High]
> Can holding vdev_mutex during a GFP_KERNEL allocation cause a deadlock here?
>
> Under memory pressure, GFP_KERNEL triggers direct reclaim, which flushes
> dirty pages to the block device. If the bio requires inline encryption, the
> block layer synchronously calls virtblk_crypto_keyslot_program() from the I/O
> submission path. That callback immediately attempts to acquire vdev_mutex,
> which is already held by the sleeping thread, causing a strict AB-BA/AA
> deadlock.
>
> [ ... ]
>> +static unsigned int get_supported_blk_key_types(u8 virtio_key_types)
>> +{
>> + unsigned int supported = 0;
>> +
>> + if (virtio_key_types & VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW)
>> + supported |= virtio_key_type_to_blk(VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW);
>> + if (virtio_key_types & VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED)
>> + supported |= virtio_key_type_to_blk(VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED);
>> +
>> + return supported;
>> +}
>
> [Severity: Medium]
> Does this construct an invalid capability bitmask by directly bitwise ORing
> raw enum values instead of shifting them?
>
> The block layer's profile->key_types_supported strictly requires a bitmask
> where support is represented as (1 << BLK_CRYPTO_KEY_TYPE_...). For
> BLK_CRYPTO_KEY_TYPE_HW_WRAPPED (value 2), the accumulated enum value sets
> bit 1, making supported equal to 3. The core block layer later checks
> 3 & (1 << 2), which evaluates to 0, incorrectly causing
> blk_crypto_config_supported() to reject hardware-wrapped keys as unsupported.
No need shifting them here.
The return value of get_supported_blk_key_types is already a bitmask. Only
when virtio_key_types contains both VIRTIO_BLK_CRYPTO_KEY_TYPE_RAW and
VIRTIO_BLK_CRYPTO_KEY_TYPE_HW_WRAPPED, the 'supported' is 3, which means
both BLK_CRYPTO_KEY_TYPE_HW_WRAPPED key and BLK_CRYPTO_KEY_TYPE_RAW are
supported, which is expected.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-08 11:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-20 12:24 [PATCH v3 0/2] FBE virtualization: inline encryption for virtio-blk guests Linlin Zhang
2026-09-20 12:24 ` [PATCH v3 1/2] virtio_blk: Add control virtqueue support Linlin Zhang
2026-09-20 12:35 ` sashiko-bot
2026-10-08 11:10 ` Linlin Zhang
2026-09-20 12:24 ` [PATCH v3 2/2] virtio_blk: add inline encryption support Linlin Zhang
2026-09-20 12:38 ` sashiko-bot
2026-10-08 11:10 ` Linlin Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox