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