* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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 2026-06-08 14:35 ` helei 0 siblings, 1 reply; 10+ 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] 10+ messages in thread
* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation 2026-06-07 2:18 ` Michael S. Tsirkin @ 2026-06-08 14:35 ` helei 2026-06-09 15:59 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: helei @ 2026-06-08 14:35 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel, arei.gonglei, zhenwei.pi 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation 2026-06-08 14:35 ` helei @ 2026-06-09 15:59 ` Michael S. Tsirkin 2026-06-16 2:50 ` helei 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2026-06-09 15:59 UTC (permalink / raw) To: helei; +Cc: qemu-devel, arei.gonglei, zhenwei.pi 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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation 2026-06-09 15:59 ` Michael S. Tsirkin @ 2026-06-16 2:50 ` helei 2026-06-16 4:37 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: helei @ 2026-06-16 2:50 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel, arei.gonglei, zhenwei.pi On 6/9/26 23:59, Michael S. Tsirkin wrote: > 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? > yes, using max_size as a fallback hard limit across all virtqueues(both dataq and ctrlq) is a very good way to close this gap without breaking backward compatibility. as for existing guests, virtio-crypto spec only defined RSA and ECDSA for asymmetric services. even an excessively large 16384 RSA key is only 2KB, so no well-behaved guest will ever be affected by a 1MB boundary on a session creation request. note that if we take this approach, we should explicitly check both the algorithm-specific limits (max_cipher_key_len/max_auth_key_len) and the global max_size boundary across all session creation requests. >>> >>> >>> 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 >>> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation 2026-06-16 2:50 ` helei @ 2026-06-16 4:37 ` Michael S. Tsirkin 0 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2026-06-16 4:37 UTC (permalink / raw) To: helei; +Cc: qemu-devel, arei.gonglei, zhenwei.pi On Tue, Jun 16, 2026 at 10:50:24AM +0800, helei wrote: > > > On 6/9/26 23:59, Michael S. Tsirkin wrote: > > 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? > > > yes, using max_size as a fallback hard limit across all virtqueues(both > dataq and ctrlq) is a very good way to close this gap without breaking > backward compatibility. > as for existing guests, virtio-crypto spec only defined RSA and ECDSA > for asymmetric services. even an excessively large 16384 RSA key is only > 2KB, so no well-behaved guest will ever be affected by a 1MB boundary on > a session creation request. > > note that if we take this approach, we should explicitly check both the > algorithm-specific limits (max_cipher_key_len/max_auth_key_len) and the > global max_size boundary across all session creation requests. why should we? one could argue alg specific limits override the max_size. > >>> > >>> > >>> 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 > >>> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-06-16 4:38 UTC | newest] Thread overview: 10+ 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 2026-06-08 14:35 ` helei 2026-06-09 15:59 ` Michael S. Tsirkin 2026-06-16 2:50 ` helei 2026-06-16 4:37 ` 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.