[PATCH 0/5] crypto virtio cleanups

[PATCH 0/5] crypto virtio cleanups

[Lukas Wunner]

Here's an assortment of trivial crypto virtio cleanups
which I accumulated while working on commit 5b553e06b321
("crypto: virtio - Drop sign/verify operations").

I've used qemu + libgcrypt backend to ascertain that all
boot-time crypto selftests still pass after these changes.
I've also verified that a KEYCTL_PKEY_ENCRYPT operation
using virtio-pkcs1-rsa produces correct output.

Thanks!

Lukas Wunner (5):
  crypto: virtio - Fix kernel-doc of virtcrypto_dev_stop()
  crypto: virtio - Simplify RSA key size caching
  crypto: virtio - Drop superfluous ctx->tfm backpointer
  crypto: virtio - Drop superfluous [as]kcipher_ctx pointer
  crypto: virtio - Drop superfluous [as]kcipher_req pointer

 .../virtio/virtio_crypto_akcipher_algs.c      | 41 ++++++++-----------
 drivers/crypto/virtio/virtio_crypto_mgr.c     |  2 +-
 .../virtio/virtio_crypto_skcipher_algs.c      | 17 ++------
 3 files changed, 21 insertions(+), 39 deletions(-)

-- 
2.43.0

[PATCH 1/5] crypto: virtio - Fix kernel-doc of virtcrypto_dev_stop()

[Lukas Wunner]

It seems the kernel-doc of virtcrypto_dev_start() was copied verbatim to
virtcrypto_dev_stop().  Fix it.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/crypto/virtio/virtio_crypto_mgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_mgr.c b/drivers/crypto/virtio/virtio_crypto_mgr.c
index 70e778aac0f2..bddbd8ebfebe 100644
--- a/drivers/crypto/virtio/virtio_crypto_mgr.c
+++ b/drivers/crypto/virtio/virtio_crypto_mgr.c
@@ -256,7 +256,7 @@ int virtcrypto_dev_start(struct virtio_crypto *vcrypto)
  * @vcrypto:    Pointer to virtio crypto device.
  *
  * Function notifies all the registered services that the virtio crypto device
- * is ready to be used.
+ * shall no longer be used.
  * To be used by virtio crypto device specific drivers.
  *
  * Return: void
-- 
2.43.0

[PATCH 2/5] crypto: virtio - Simplify RSA key size caching

[Lukas Wunner]

When setting a public or private RSA key, the integer n is cached in the
transform context virtio_crypto_akcipher_ctx -- with the sole purpose of
calculating the key size from it in virtio_crypto_rsa_max_size().
It looks like this was copy-pasted from crypto/rsa.c.

Cache the key size directly instead of the integer n, thus simplifying
the code and reducing the memory footprint.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 .../virtio/virtio_crypto_akcipher_algs.c      | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 48fee07b7e51..7fdf32c79909 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -21,7 +21,7 @@
 #include "virtio_crypto_common.h"
 
 struct virtio_crypto_rsa_ctx {
-	MPI n;
+	unsigned int key_size;
 };
 
 struct virtio_crypto_akcipher_ctx {
@@ -352,10 +352,7 @@ static int virtio_crypto_rsa_set_key(struct crypto_akcipher *tfm,
 	int node = virtio_crypto_get_current_node();
 	uint32_t keytype;
 	int ret;
-
-	/* mpi_free will test n, just free it. */
-	mpi_free(rsa_ctx->n);
-	rsa_ctx->n = NULL;
+	MPI n;
 
 	if (private) {
 		keytype = VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PRIVATE;
@@ -368,10 +365,13 @@ static int virtio_crypto_rsa_set_key(struct crypto_akcipher *tfm,
 	if (ret)
 		return ret;
 
-	rsa_ctx->n = mpi_read_raw_data(rsa_key.n, rsa_key.n_sz);
-	if (!rsa_ctx->n)
+	n = mpi_read_raw_data(rsa_key.n, rsa_key.n_sz);
+	if (!n)
 		return -ENOMEM;
 
+	rsa_ctx->key_size = mpi_get_size(n);
+	mpi_free(n);
+
 	if (!ctx->vcrypto) {
 		vcrypto = virtcrypto_get_dev_node(node, VIRTIO_CRYPTO_SERVICE_AKCIPHER,
 						VIRTIO_CRYPTO_AKCIPHER_RSA);
@@ -442,7 +442,7 @@ static unsigned int virtio_crypto_rsa_max_size(struct crypto_akcipher *tfm)
 	struct virtio_crypto_akcipher_ctx *ctx = akcipher_tfm_ctx(tfm);
 	struct virtio_crypto_rsa_ctx *rsa_ctx = &ctx->rsa_ctx;
 
-	return mpi_get_size(rsa_ctx->n);
+	return rsa_ctx->key_size;
 }
 
 static int virtio_crypto_rsa_init_tfm(struct crypto_akcipher *tfm)
@@ -460,12 +460,9 @@ static int virtio_crypto_rsa_init_tfm(struct crypto_akcipher *tfm)
 static void virtio_crypto_rsa_exit_tfm(struct crypto_akcipher *tfm)
 {
 	struct virtio_crypto_akcipher_ctx *ctx = akcipher_tfm_ctx(tfm);
-	struct virtio_crypto_rsa_ctx *rsa_ctx = &ctx->rsa_ctx;
 
 	virtio_crypto_alg_akcipher_close_session(ctx);
 	virtcrypto_dev_put(ctx->vcrypto);
-	mpi_free(rsa_ctx->n);
-	rsa_ctx->n = NULL;
 }
 
 static struct virtio_crypto_akcipher_algo virtio_crypto_akcipher_algs[] = {
-- 
2.43.0

[PATCH 3/5] crypto: virtio - Drop superfluous ctx->tfm backpointer

[Lukas Wunner]

struct virtio_crypto_[as]kcipher_ctx contains a backpointer to struct
crypto_[as]kcipher which is superfluous in two ways:

First, it's not used anywhere.  Second, the context is embedded into
struct crypto_tfm, so one could just use container_of() to get from the
context to crypto_tfm and from there to crypto_[as]kcipher.

Drop the superfluous backpointer.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 5 -----
 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c | 4 ----
 2 files changed, 9 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 7fdf32c79909..aa8255786d6c 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -26,7 +26,6 @@ struct virtio_crypto_rsa_ctx {
 
 struct virtio_crypto_akcipher_ctx {
 	struct virtio_crypto *vcrypto;
-	struct crypto_akcipher *tfm;
 	bool session_valid;
 	__u64 session_id;
 	union {
@@ -447,10 +446,6 @@ static unsigned int virtio_crypto_rsa_max_size(struct crypto_akcipher *tfm)
 
 static int virtio_crypto_rsa_init_tfm(struct crypto_akcipher *tfm)
 {
-	struct virtio_crypto_akcipher_ctx *ctx = akcipher_tfm_ctx(tfm);
-
-	ctx->tfm = tfm;
-
 	akcipher_set_reqsize(tfm,
 			     sizeof(struct virtio_crypto_akcipher_request));
 
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index 23c41d87d835..495fc655a51c 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -17,7 +17,6 @@
 
 struct virtio_crypto_skcipher_ctx {
 	struct virtio_crypto *vcrypto;
-	struct crypto_skcipher *tfm;
 
 	struct virtio_crypto_sym_session_info enc_sess_info;
 	struct virtio_crypto_sym_session_info dec_sess_info;
@@ -515,10 +514,7 @@ static int virtio_crypto_skcipher_decrypt(struct skcipher_request *req)
 
 static int virtio_crypto_skcipher_init(struct crypto_skcipher *tfm)
 {
-	struct virtio_crypto_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-
 	crypto_skcipher_set_reqsize(tfm, sizeof(struct virtio_crypto_sym_request));
-	ctx->tfm = tfm;
 
 	return 0;
 }
-- 
2.43.0

[PATCH 4/5] crypto: virtio - Drop superfluous [as]kcipher_ctx pointer

[Lukas Wunner]

The request context virtio_crypto_{akcipher,sym}_request contains a
pointer to the transform context virtio_crypto_[as]kcipher_ctx.

The pointer is superfluous as it can be calculated with the cheap
crypto_akcipher_reqtfm() + akcipher_tfm_ctx() and
crypto_skcipher_reqtfm() + crypto_skcipher_ctx() combos.

Drop the superfluous pointer.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 8 ++++----
 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c | 5 +----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index aa8255786d6c..ac8eb3d07c93 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -35,7 +35,6 @@ struct virtio_crypto_akcipher_ctx {
 
 struct virtio_crypto_akcipher_request {
 	struct virtio_crypto_request base;
-	struct virtio_crypto_akcipher_ctx *akcipher_ctx;
 	struct akcipher_request *akcipher_req;
 	void *src_buf;
 	void *dst_buf;
@@ -212,7 +211,8 @@ static int virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
 static int __virtio_crypto_akcipher_do_req(struct virtio_crypto_akcipher_request *vc_akcipher_req,
 		struct akcipher_request *req, struct data_queue *data_vq)
 {
-	struct virtio_crypto_akcipher_ctx *ctx = vc_akcipher_req->akcipher_ctx;
+	struct crypto_akcipher *atfm = crypto_akcipher_reqtfm(req);
+	struct virtio_crypto_akcipher_ctx *ctx = akcipher_tfm_ctx(atfm);
 	struct virtio_crypto_request *vc_req = &vc_akcipher_req->base;
 	struct virtio_crypto *vcrypto = ctx->vcrypto;
 	struct virtio_crypto_op_data_req *req_data = vc_req->req_data;
@@ -272,7 +272,8 @@ static int virtio_crypto_rsa_do_req(struct crypto_engine *engine, void *vreq)
 	struct akcipher_request *req = container_of(vreq, struct akcipher_request, base);
 	struct virtio_crypto_akcipher_request *vc_akcipher_req = akcipher_request_ctx(req);
 	struct virtio_crypto_request *vc_req = &vc_akcipher_req->base;
-	struct virtio_crypto_akcipher_ctx *ctx = vc_akcipher_req->akcipher_ctx;
+	struct crypto_akcipher *atfm = crypto_akcipher_reqtfm(req);
+	struct virtio_crypto_akcipher_ctx *ctx = akcipher_tfm_ctx(atfm);
 	struct virtio_crypto *vcrypto = ctx->vcrypto;
 	struct data_queue *data_vq = vc_req->dataq;
 	struct virtio_crypto_op_header *header;
@@ -318,7 +319,6 @@ static int virtio_crypto_rsa_req(struct akcipher_request *req, uint32_t opcode)
 
 	vc_req->dataq = data_vq;
 	vc_req->alg_cb = virtio_crypto_dataq_akcipher_callback;
-	vc_akcipher_req->akcipher_ctx = ctx;
 	vc_akcipher_req->akcipher_req = req;
 	vc_akcipher_req->opcode = opcode;
 
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index 495fc655a51c..e2a481a29b77 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -27,7 +27,6 @@ struct virtio_crypto_sym_request {
 
 	/* Cipher or aead */
 	uint32_t type;
-	struct virtio_crypto_skcipher_ctx *skcipher_ctx;
 	struct skcipher_request *skcipher_req;
 	uint8_t *iv;
 	/* Encryption? */
@@ -324,7 +323,7 @@ __virtio_crypto_skcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req,
 		struct data_queue *data_vq)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
-	struct virtio_crypto_skcipher_ctx *ctx = vc_sym_req->skcipher_ctx;
+	struct virtio_crypto_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct virtio_crypto_request *vc_req = &vc_sym_req->base;
 	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
 	struct virtio_crypto *vcrypto = ctx->vcrypto;
@@ -480,7 +479,6 @@ static int virtio_crypto_skcipher_encrypt(struct skcipher_request *req)
 
 	vc_req->dataq = data_vq;
 	vc_req->alg_cb = virtio_crypto_dataq_sym_callback;
-	vc_sym_req->skcipher_ctx = ctx;
 	vc_sym_req->skcipher_req = req;
 	vc_sym_req->encrypt = true;
 
@@ -505,7 +503,6 @@ static int virtio_crypto_skcipher_decrypt(struct skcipher_request *req)
 
 	vc_req->dataq = data_vq;
 	vc_req->alg_cb = virtio_crypto_dataq_sym_callback;
-	vc_sym_req->skcipher_ctx = ctx;
 	vc_sym_req->skcipher_req = req;
 	vc_sym_req->encrypt = false;
 
-- 
2.43.0

[PATCH 5/5] crypto: virtio - Drop superfluous [as]kcipher_req pointer

[Lukas Wunner]

The request context virtio_crypto_{akcipher,sym}_request contains a
pointer to the [as]kcipher_request itself.

The pointer is superfluous as it can be calculated with container_of().

Drop the superfluous pointer.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
I've considered introducing a static inline to <crypto/[as]kcipher.h>
to get from the request context to the request, but these two seem to be
the only occurrences in the tree which would need it, so I figured it's
probably not worth it.  If anyone disagrees, please speak up.

 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 9 ++++-----
 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c | 8 +++-----
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index ac8eb3d07c93..2e44915c9f23 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -35,7 +35,6 @@ struct virtio_crypto_akcipher_ctx {
 
 struct virtio_crypto_akcipher_request {
 	struct virtio_crypto_request base;
-	struct akcipher_request *akcipher_req;
 	void *src_buf;
 	void *dst_buf;
 	uint32_t opcode;
@@ -67,7 +66,9 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
 {
 	struct virtio_crypto_akcipher_request *vc_akcipher_req =
 		container_of(vc_req, struct virtio_crypto_akcipher_request, base);
-	struct akcipher_request *akcipher_req;
+	struct akcipher_request *akcipher_req =
+		container_of((void *)vc_akcipher_req, struct akcipher_request,
+			     __ctx);
 	int error;
 
 	switch (vc_req->status) {
@@ -86,8 +87,7 @@ static void virtio_crypto_dataq_akcipher_callback(struct virtio_crypto_request *
 		break;
 	}
 
-	akcipher_req = vc_akcipher_req->akcipher_req;
-	/* actual length maybe less than dst buffer */
+	/* actual length may be less than dst buffer */
 	akcipher_req->dst_len = len - sizeof(vc_req->status);
 	sg_copy_from_buffer(akcipher_req->dst, sg_nents(akcipher_req->dst),
 			    vc_akcipher_req->dst_buf, akcipher_req->dst_len);
@@ -319,7 +319,6 @@ static int virtio_crypto_rsa_req(struct akcipher_request *req, uint32_t opcode)
 
 	vc_req->dataq = data_vq;
 	vc_req->alg_cb = virtio_crypto_dataq_akcipher_callback;
-	vc_akcipher_req->akcipher_req = req;
 	vc_akcipher_req->opcode = opcode;
 
 	return crypto_transfer_akcipher_request_to_engine(data_vq->engine, req);
diff --git a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
index e2a481a29b77..1b3fb21a2a7d 100644
--- a/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_skcipher_algs.c
@@ -27,7 +27,6 @@ struct virtio_crypto_sym_request {
 
 	/* Cipher or aead */
 	uint32_t type;
-	struct skcipher_request *skcipher_req;
 	uint8_t *iv;
 	/* Encryption? */
 	bool encrypt;
@@ -55,7 +54,9 @@ static void virtio_crypto_dataq_sym_callback
 {
 	struct virtio_crypto_sym_request *vc_sym_req =
 		container_of(vc_req, struct virtio_crypto_sym_request, base);
-	struct skcipher_request *ablk_req;
+	struct skcipher_request *ablk_req =
+		container_of((void *)vc_sym_req, struct skcipher_request,
+			     __ctx);
 	int error;
 
 	/* Finish the encrypt or decrypt process */
@@ -75,7 +76,6 @@ static void virtio_crypto_dataq_sym_callback
 			error = -EIO;
 			break;
 		}
-		ablk_req = vc_sym_req->skcipher_req;
 		virtio_crypto_skcipher_finalize_req(vc_sym_req,
 							ablk_req, error);
 	}
@@ -479,7 +479,6 @@ static int virtio_crypto_skcipher_encrypt(struct skcipher_request *req)
 
 	vc_req->dataq = data_vq;
 	vc_req->alg_cb = virtio_crypto_dataq_sym_callback;
-	vc_sym_req->skcipher_req = req;
 	vc_sym_req->encrypt = true;
 
 	return crypto_transfer_skcipher_request_to_engine(data_vq->engine, req);
@@ -503,7 +502,6 @@ static int virtio_crypto_skcipher_decrypt(struct skcipher_request *req)
 
 	vc_req->dataq = data_vq;
 	vc_req->alg_cb = virtio_crypto_dataq_sym_callback;
-	vc_sym_req->skcipher_req = req;
 	vc_sym_req->encrypt = false;
 
 	return crypto_transfer_skcipher_request_to_engine(data_vq->engine, req);
-- 
2.43.0

Re: [PATCH 0/5] crypto virtio cleanups

[zhenwei pi]

This series looks good to me, thanks!

Reviewed-by: zhenwei pi <pizhenwei@bytedance.com>

On 2/3/25 21:37, Lukas Wunner wrote:
> Here's an assortment of trivial crypto virtio cleanups
> which I accumulated while working on commit 5b553e06b321
> ("crypto: virtio - Drop sign/verify operations").
> 
> I've used qemu + libgcrypt backend to ascertain that all
> boot-time crypto selftests still pass after these changes.
> I've also verified that a KEYCTL_PKEY_ENCRYPT operation
> using virtio-pkcs1-rsa produces correct output.
> 
> Thanks!
> 
> Lukas Wunner (5):
>    crypto: virtio - Fix kernel-doc of virtcrypto_dev_stop()
>    crypto: virtio - Simplify RSA key size caching
>    crypto: virtio - Drop superfluous ctx->tfm backpointer
>    crypto: virtio - Drop superfluous [as]kcipher_ctx pointer
>    crypto: virtio - Drop superfluous [as]kcipher_req pointer
> 
>   .../virtio/virtio_crypto_akcipher_algs.c      | 41 ++++++++-----------
>   drivers/crypto/virtio/virtio_crypto_mgr.c     |  2 +-
>   .../virtio/virtio_crypto_skcipher_algs.c      | 17 ++------
>   3 files changed, 21 insertions(+), 39 deletions(-)
> 

Re: [PATCH 0/5] crypto virtio cleanups

[Herbert Xu]

On Mon, Feb 03, 2025 at 02:37:00PM +0100, Lukas Wunner wrote:
> Here's an assortment of trivial crypto virtio cleanups
> which I accumulated while working on commit 5b553e06b321
> ("crypto: virtio - Drop sign/verify operations").
> 
> I've used qemu + libgcrypt backend to ascertain that all
> boot-time crypto selftests still pass after these changes.
> I've also verified that a KEYCTL_PKEY_ENCRYPT operation
> using virtio-pkcs1-rsa produces correct output.
> 
> Thanks!
> 
> Lukas Wunner (5):
>   crypto: virtio - Fix kernel-doc of virtcrypto_dev_stop()
>   crypto: virtio - Simplify RSA key size caching
>   crypto: virtio - Drop superfluous ctx->tfm backpointer
>   crypto: virtio - Drop superfluous [as]kcipher_ctx pointer
>   crypto: virtio - Drop superfluous [as]kcipher_req pointer
> 
>  .../virtio/virtio_crypto_akcipher_algs.c      | 41 ++++++++-----------
>  drivers/crypto/virtio/virtio_crypto_mgr.c     |  2 +-
>  .../virtio/virtio_crypto_skcipher_algs.c      | 17 ++------
>  3 files changed, 21 insertions(+), 39 deletions(-)
> 
> -- 
> 2.43.0

All applied.  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