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