[PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[helei]

The virtio-crypto spec does not dictate a maximum length limit for 
asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
the linux kernels's internal limit for akcipher keys (see 
keyctl framework and the add_key syscall).
Maybe we should update the virtio-spec and add a max_akcipher_key_len
field for virtio crypto devices.

helei (1):
  hw/virtio-crypto: enforce max akcipher key length

 hw/virtio/virtio-crypto.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.43.0

[PATCH] hw/virtio-crypto: enforce max akcipher key length

[helei]

Introduce VIRTIO_CRYPTO_MAX_AKCIPHER_KEY_LEN. We set this hard
limit to 1MB, which mirrors the linux kernel's internal payload
restriction for the 'add_key' syscall.

Signed-off-by: helei <lhestz@163.com>
---
 hw/virtio/virtio-crypto.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 6fceb39681..06be93a0ac 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -25,6 +25,13 @@
 #include "system/cryptodev-vhost.h"
 
 #define VIRTIO_CRYPTO_VM_VERSION 1
+/*
+ * The virtio-crypto spec does not limit akcipher key lengths. To prevent
+ * guest-introduced OOM attacks via excessive host memory allocation, we
+ * enforce a 1MB limit. This aligns with the linux kernel's internal max
+ * payload limit for the add_key syscall.
+ */
+#define VIRTIO_CRYPTO_MAX_AKCIPHER_KEY_LEN ((1024 * 1024) - 1)
 
 typedef struct VirtIOCryptoSessionReq {
     VirtIODevice *vdev;
@@ -216,6 +223,12 @@ virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
         return -VIRTIO_CRYPTO_NOTSUPP;
     }
 
+    if (keylen > VIRTIO_CRYPTO_MAX_AKCIPHER_KEY_LEN) {
+        error_report("virtio-crypto length of akcipher key is too large: %u",
+                     keylen);
+        return -VIRTIO_CRYPTO_ERR;
+    }
+
     if (keylen) {
         asym_info->key = g_malloc(keylen);
         if (iov_to_buf(iov, out_num, 0, asym_info->key, keylen) != keylen) {
-- 
2.43.0

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[Michael S. Tsirkin]

On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
> The virtio-crypto spec does not dictate a maximum length limit for 
> asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
> the linux kernels's internal limit for akcipher keys (see 
> keyctl framework and the add_key syscall).

We have max_size - doesn't that apply?
backends/cryptodev-builtin.c actually sets it:
backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;



> Maybe we should update the virtio-spec and add a max_akcipher_key_len
> field for virtio crypto devices.

maybe

> 
> helei (1):
>   hw/virtio-crypto: enforce max akcipher key length
> 
>  hw/virtio/virtio-crypto.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> -- 
> 2.43.0

Re: [PATCH] hw/virtio-crypto: enforce max akcipher key length

[Manos Pitsidianakis]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On Sat, 6 Jun 2026, 10:52 helei, <lhestz@163.com> wrote:

> Introduce VIRTIO_CRYPTO_MAX_AKCIPHER_KEY_LEN. We set this hard
> limit to 1MB, which mirrors the linux kernel's internal payload
> restriction for the 'add_key' syscall.
>
> Signed-off-by: helei <lhestz@163.com>
> ---
>  hw/virtio/virtio-crypto.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> index 6fceb39681..06be93a0ac 100644
> --- a/hw/virtio/virtio-crypto.c
> +++ b/hw/virtio/virtio-crypto.c
> @@ -25,6 +25,13 @@
>  #include "system/cryptodev-vhost.h"
>
>  #define VIRTIO_CRYPTO_VM_VERSION 1
> +/*
> + * The virtio-crypto spec does not limit akcipher key lengths. To prevent
> + * guest-introduced OOM attacks via excessive host memory allocation, we
> + * enforce a 1MB limit. This aligns with the linux kernel's internal max
> + * payload limit for the add_key syscall.
> + */
> +#define VIRTIO_CRYPTO_MAX_AKCIPHER_KEY_LEN ((1024 * 1024) - 1)
>
>  typedef struct VirtIOCryptoSessionReq {
>      VirtIODevice *vdev;
> @@ -216,6 +223,12 @@ virtio_crypto_create_asym_session(VirtIOCrypto
> *vcrypto,
>          return -VIRTIO_CRYPTO_NOTSUPP;
>      }
>
> +    if (keylen > VIRTIO_CRYPTO_MAX_AKCIPHER_KEY_LEN) {
> +        error_report("virtio-crypto length of akcipher key is too large:
> %u",
> +                     keylen);
> +        return -VIRTIO_CRYPTO_ERR;
> +    }
> +
>      if (keylen) {
>          asym_info->key = g_malloc(keylen);
>

Unrelated to this patch, but this g_malloc could be g_try_malloc.

         if (iov_to_buf(iov, out_num, 0, asym_info->key, keylen) != keylen)
> {
> --
> 2.43.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 2581 bytes --]

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[helei]

On 6/6/26 16:37, Michael S. Tsirkin wrote:
> On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
>> The virtio-crypto spec does not dictate a maximum length limit for
>> asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
>> the linux kernels's internal limit for akcipher keys (see
>> keyctl framework and the add_key syscall).
> We have max_size - doesn't that apply?
> backends/cryptodev-builtin.c actually sets it:
> backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
> backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;

Thanks for your review!  I have verified via testing that all processing 
requests in the dataq are strictly

bounded by max_size, but session creation requests in the ctrlq are not.

>
>> Maybe we should update the virtio-spec and add a max_akcipher_key_len
>> field for virtio crypto devices.
> maybe
>
>> helei (1):
>>    hw/virtio-crypto: enforce max akcipher key length
>>
>>   hw/virtio/virtio-crypto.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> -- 
>> 2.43.0

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[Michael S. Tsirkin]

On Sun, Jun 07, 2026 at 08:15:19AM +0800, helei wrote:
> 
> On 6/6/26 16:37, Michael S. Tsirkin wrote:
> > On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
> > > The virtio-crypto spec does not dictate a maximum length limit for
> > > asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
> > > the linux kernels's internal limit for akcipher keys (see
> > > keyctl framework and the add_key syscall).
> > We have max_size - doesn't that apply?
> > backends/cryptodev-builtin.c actually sets it:
> > backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
> > backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;
> 
> Thanks for your review!  I have verified via testing that all processing
> requests in the dataq are strictly
> 
> bounded by max_size, but session creation requests in the ctrlq are not.



well if we read the spec it's vague

  max_size is defined as "the maximum size of the variable-length parameters of data operation of
  each crypto request's content." and
  The driver SHOULD read max_size to discover the maximum size of the variable-length parameters of
  data operation of the crypto request's content


  so data operation.

  however:

  "The device MUST set max_size to show the
  maximum size of crypto request the device supports".

  seems to cover all requests?



btw vhost user sets max_size to max u64 - is that sane?

  

> > 
> > > Maybe we should update the virtio-spec and add a max_akcipher_key_len
> > > field for virtio crypto devices.
> > maybe
> > 
> > > helei (1):
> > >    hw/virtio-crypto: enforce max akcipher key length
> > > 
> > >   hw/virtio/virtio-crypto.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > -- 
> > > 2.43.0

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[helei]

On 6/7/26 10:18, Michael S. Tsirkin wrote:
> On Sun, Jun 07, 2026 at 08:15:19AM +0800, helei wrote:
>>
>> On 6/6/26 16:37, Michael S. Tsirkin wrote:
>>> On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
>>>> The virtio-crypto spec does not dictate a maximum length limit for
>>>> asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
>>>> the linux kernels's internal limit for akcipher keys (see
>>>> keyctl framework and the add_key syscall).
>>> We have max_size - doesn't that apply?
>>> backends/cryptodev-builtin.c actually sets it:
>>> backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
>>> backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;
>>
>> Thanks for your review!  I have verified via testing that all processing
>> requests in the dataq are strictly
>>
>> bounded by max_size, but session creation requests in the ctrlq are not.
> 
> 
> 
> well if we read the spec it's vague
> 
>   max_size is defined as "the maximum size of the variable-length parameters of data operation of
>   each crypto request's content." and
>   The driver SHOULD read max_size to discover the maximum size of the variable-length parameters of
>   data operation of the crypto request's content
> 
> 
>   so data operation.
> 
>   however:
> 
>   "The device MUST set max_size to show the
>   maximum size of crypto request the device supports".
> 
>   seems to cover all requests?
> 
the original architectural intent was almost certainly for max_size to
govern data-plane payloads, while control messages are handled by their
own independent limits (max_cipher_key_len and max_auth_key_len).
both upstream linux guest driver (virtio_crypto_skcipher_algs.c) and
qemu's builtin-backend actually implement it exactly this way:
- data path enforces max_size by checking it against payload length
(note that cryptodev-builtin recently capped this to 1MB via CVE-2025-14876)
- control path completely ignores max_size when creating sessions

> 
> 
> btw vhost user sets max_size to max u64 - is that sane?
> 
this is sane for an out-of-process data plane from a transport
perspective, but it creates a practical catch when backed by a dpdk
implementation:
- in dpdk vhost_crypto.c, vhost_crypto_check_cipher_request() enforces a
strict check: src_data_len <= RTE_MBUF_DEFAULT_BUF_SIZE (2176 bytes)
- the guest kernel driver simply wraps the incoming sg list and passes
it to virtqueue directly.
this creates a tricky situation for users: because vhost-user claims
max_size is max_u64, the users has no way of knowing this 2KB ceiling
exists unless they explicitly know the host backend is dpdk and look at
its source code.

>   
> 
>>>
>>>> Maybe we should update the virtio-spec and add a max_akcipher_key_len
>>>> field for virtio crypto devices.
>>> maybe
>>>
>>>> helei (1):
>>>>    hw/virtio-crypto: enforce max akcipher key length
>>>>
>>>>   hw/virtio/virtio-crypto.c | 13 +++++++++++++
>>>>   1 file changed, 13 insertions(+)
>>>>
>>>> -- 
>>>> 2.43.0
> 

Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

[Michael S. Tsirkin]

On Mon, Jun 08, 2026 at 10:35:14PM +0800, helei wrote:
> 
> 
> On 6/7/26 10:18, Michael S. Tsirkin wrote:
> > On Sun, Jun 07, 2026 at 08:15:19AM +0800, helei wrote:
> >>
> >> On 6/6/26 16:37, Michael S. Tsirkin wrote:
> >>> On Sat, Jun 06, 2026 at 03:47:55PM +0800, helei wrote:
> >>>> The virtio-crypto spec does not dictate a maximum length limit for
> >>>> asymmetric cipher (akcipher) keys. We added a hard limit which mirrors
> >>>> the linux kernels's internal limit for akcipher keys (see
> >>>> keyctl framework and the add_key syscall).
> >>> We have max_size - doesn't that apply?
> >>> backends/cryptodev-builtin.c actually sets it:
> >>> backends/cryptodev-builtin.c:#define CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE  (1024 * 1024)
> >>> backends/cryptodev-builtin.c:    backend->conf.max_size = CRYPTODEV_BUITLIN_MAX_REQUEST_SIZE;
> >>
> >> Thanks for your review!  I have verified via testing that all processing
> >> requests in the dataq are strictly
> >>
> >> bounded by max_size, but session creation requests in the ctrlq are not.
> > 
> > 
> > 
> > well if we read the spec it's vague
> > 
> >   max_size is defined as "the maximum size of the variable-length parameters of data operation of
> >   each crypto request's content." and
> >   The driver SHOULD read max_size to discover the maximum size of the variable-length parameters of
> >   data operation of the crypto request's content
> > 
> > 
> >   so data operation.
> > 
> >   however:
> > 
> >   "The device MUST set max_size to show the
> >   maximum size of crypto request the device supports".
> > 
> >   seems to cover all requests?
> > 
> the original architectural intent was almost certainly for max_size to
> govern data-plane payloads, while control messages are handled by their
> own independent limits (max_cipher_key_len and max_auth_key_len).
> both upstream linux guest driver (virtio_crypto_skcipher_algs.c) and
> qemu's builtin-backend actually implement it exactly this way:
> - data path enforces max_size by checking it against payload length
> (note that cryptodev-builtin recently capped this to 1MB via CVE-2025-14876)
> - control path completely ignores max_size when creating sessions

we can add akcipher key length feature but what to do about existing
guests?

could max_size be a way to fix it?

> > 
> > 
> > btw vhost user sets max_size to max u64 - is that sane?
> > 
> this is sane for an out-of-process data plane from a transport
> perspective, but it creates a practical catch when backed by a dpdk
> implementation:
> - in dpdk vhost_crypto.c, vhost_crypto_check_cipher_request() enforces a
> strict check: src_data_len <= RTE_MBUF_DEFAULT_BUF_SIZE (2176 bytes)
> - the guest kernel driver simply wraps the incoming sg list and passes
> it to virtqueue directly.
> this creates a tricky situation for users: because vhost-user claims
> max_size is max_u64, the users has no way of knowing this 2KB ceiling
> exists unless they explicitly know the host backend is dpdk and look at
> its source code.

worth fixing maybe.

> >   
> > 
> >>>
> >>>> Maybe we should update the virtio-spec and add a max_akcipher_key_len
> >>>> field for virtio crypto devices.
> >>> maybe
> >>>
> >>>> helei (1):
> >>>>    hw/virtio-crypto: enforce max akcipher key length
> >>>>
> >>>>   hw/virtio/virtio-crypto.c | 13 +++++++++++++
> >>>>   1 file changed, 13 insertions(+)
> >>>>
> >>>> -- 
> >>>> 2.43.0
> >