Re: [PATCH 0/1] hw/virtio-crypto: add akcipher keylen validation

["Michael S. Tsirkin" <mst@redhat.com>]

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