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