All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation
@ 2026-06-06  7:47 helei
  2026-06-06  7:47 ` [PATCH] hw/virtio-crypto: enforce max akcipher key length helei
  2026-06-06  8:37 ` [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation Michael S. Tsirkin
  0 siblings, 2 replies; 6+ messages in thread
From: helei @ 2026-06-06  7:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: arei.gonglei, zhenwei.pi, mst, 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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] hw/virtio-crypto: enforce max akcipher key length
  2026-06-06  7:47 [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation helei
@ 2026-06-06  7:47 ` helei
  2026-06-06  8:59   ` Manos Pitsidianakis
  2026-06-06  8:37 ` [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation Michael S. Tsirkin
  1 sibling, 1 reply; 6+ messages in thread
From: helei @ 2026-06-06  7:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: arei.gonglei, zhenwei.pi, mst, 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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation
  2026-06-06  7:47 [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation helei
  2026-06-06  7:47 ` [PATCH] hw/virtio-crypto: enforce max akcipher key length helei
@ 2026-06-06  8:37 ` Michael S. Tsirkin
  2026-06-07  0:15   ` helei
  1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2026-06-06  8:37 UTC (permalink / raw)
  To: helei; +Cc: qemu-devel, arei.gonglei, zhenwei.pi

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw/virtio-crypto: enforce max akcipher key length
  2026-06-06  7:47 ` [PATCH] hw/virtio-crypto: enforce max akcipher key length helei
@ 2026-06-06  8:59   ` Manos Pitsidianakis
  0 siblings, 0 replies; 6+ messages in thread
From: Manos Pitsidianakis @ 2026-06-06  8:59 UTC (permalink / raw)
  To: helei
  Cc: open list:ARM SMMU <qemu-arm@nongnu.org>, , Gonglei,
	zhenwei.pi, Michael S. Tsirkin

[-- 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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation
  2026-06-06  8:37 ` [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation Michael S. Tsirkin
@ 2026-06-07  0:15   ` helei
  2026-06-07  2:18     ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: helei @ 2026-06-07  0:15 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, arei.gonglei, zhenwei.pi


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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation
  2026-06-07  0:15   ` helei
@ 2026-06-07  2:18     ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2026-06-07  2:18 UTC (permalink / raw)
  To: helei; +Cc: qemu-devel, arei.gonglei, zhenwei.pi

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-07  2:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-06  7:47 [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation helei
2026-06-06  7:47 ` [PATCH] hw/virtio-crypto: enforce max akcipher key length helei
2026-06-06  8:59   ` Manos Pitsidianakis
2026-06-06  8:37 ` [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation Michael S. Tsirkin
2026-06-07  0:15   ` helei
2026-06-07  2:18     ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.