On 6/16/26 12:37, Michael S. Tsirkin wrote:
> 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.
>
fine, just leave symmetric-ciphers entirely as-is, and use max_size as
the hard ceiling ceiling specifically for akcipher session creations
requests. I will update the patch to V2.
>
>>>>>
>>>>>
>>>>> 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
>>>>>
