[PATCH] crypto: virtio: validate akcipher completion length

[PATCH] crypto: virtio: validate akcipher completion length

[Jérémy Jean]

The device controls the used length returned for an akcipher request.
Subtracting the status byte without validating that length can underflow
dst_len, while accepting a payload larger than the submitted destination
can make sg_copy_from_buffer() read past the response buffer.

Reject malformed completion lengths before updating dst_len or copying the
response.

Fixes: a36bd0ad9fbf ("virtio-crypto: adjust dst_len at ops callback")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index d8d452cac391..886032abae80 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -69,6 +69,7 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
 	struct akcipher_request *akcipher_req =
 		container_of((void *)vc_akcipher_req, struct akcipher_request,
 			     __ctx);
+	unsigned int dst_len;
 	int error;
 
 	switch (vc_req->status) {
@@ -88,9 +89,19 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
 	}
 
 	/* actual length may be less than dst buffer */
-	akcipher_req->dst_len = len - sizeof(vc_req->status);
+	if (len < (int)sizeof(vc_req->status)) {
+		error = -EIO;
+		goto out;
+	}
+	dst_len = len - sizeof(vc_req->status);
+	if (dst_len > akcipher_req->dst_len) {
+		error = -EIO;
+		goto out;
+	}
+	akcipher_req->dst_len = dst_len;
 	sg_copy_from_buffer(akcipher_req->dst, sg_nents(akcipher_req->dst),
 			    vc_akcipher_req->dst_buf, akcipher_req->dst_len);
+out:
 	virtio_crypto_akcipher_finalize_req(vc_akcipher_req, akcipher_req, error);
 }
 
-- 
2.47.3

Re: [PATCH] crypto: virtio: validate akcipher completion length

[Herbert Xu]

On Fri, Aug 21, 2026 at 09:55:12AM +0000, Jérémy Jean wrote:
> The device controls the used length returned for an akcipher request.
> Subtracting the status byte without validating that length can underflow
> dst_len, while accepting a payload larger than the submitted destination
> can make sg_copy_from_buffer() read past the response buffer.
> 
> Reject malformed completion lengths before updating dst_len or copying the
> response.
> 
> Fixes: a36bd0ad9fbf ("virtio-crypto: adjust dst_len at ops callback")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> index d8d452cac391..886032abae80 100644
> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> @@ -69,6 +69,7 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
>  	struct akcipher_request *akcipher_req =
>  		container_of((void *)vc_akcipher_req, struct akcipher_request,
>  			     __ctx);
> +	unsigned int dst_len;
>  	int error;
>  
>  	switch (vc_req->status) {
> @@ -88,9 +89,19 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
>  	}
>  
>  	/* actual length may be less than dst buffer */
> -	akcipher_req->dst_len = len - sizeof(vc_req->status);

This appears to have already been fixed by

commit f77a956f6a19f9463ef1527c9d0cda50dded6b92
Author: Bryam Vargas <hexlabsecurity@proton.me>
Date:   Mon Jun 22 01:52:15 2026 -0500

    crypto: virtio - bound the akcipher result length

Please check that commit and see if it's sufficient or not.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: virtio: validate akcipher completion length

[Jérémy Jean]

On 2026-09-08 11:20, Herbert Xu wrote:
> On Fri, Aug 21, 2026 at 09:55:12AM +0000, Jérémy Jean wrote:
>> The device controls the used length returned for an akcipher request.
>> Subtracting the status byte without validating that length can 
>> underflow
>> dst_len, while accepting a payload larger than the submitted 
>> destination
>> can make sg_copy_from_buffer() read past the response buffer.
>> 
>> Reject malformed completion lengths before updating dst_len or copying 
>> the
>> response.
>> 
>> Fixes: a36bd0ad9fbf ("virtio-crypto: adjust dst_len at ops callback")
>> Assisted-by: Codex:gpt-5
>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> ---
>>  drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 13 
>> ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c 
>> b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
>> index d8d452cac391..886032abae80 100644
>> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
>> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
>> @@ -69,6 +69,7 @@ static void 
>> virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
>>  	struct akcipher_request *akcipher_req =
>>  		container_of((void *)vc_akcipher_req, struct akcipher_request,
>>  			     __ctx);
>> +	unsigned int dst_len;
>>  	int error;
>> 
>>  	switch (vc_req->status) {
>> @@ -88,9 +89,19 @@ static void 
>> virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
>>  	}
>> 
>>  	/* actual length may be less than dst buffer */
>> -	akcipher_req->dst_len = len - sizeof(vc_req->status);
> 
> This appears to have already been fixed by
> 
> commit f77a956f6a19f9463ef1527c9d0cda50dded6b92
> Author: Bryam Vargas <hexlabsecurity@proton.me>
> Date:   Mon Jun 22 01:52:15 2026 -0500
> 
>     crypto: virtio - bound the akcipher result length
> 
> Please check that commit and see if it's sufficient or not.
> 
> Thanks,

Hello,

Indeed, this fixes the bug.
Thanks for the pointer, I missed that patch.

Regards,
Jérémy