Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
Hi zhenwei,
url:
https://github.com/intel-lab-lkp/linux/commits/zhenwei-pi/virtio-crypto-Improve-performance/20220424-184732
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: i386-randconfig-m021
(https://download.01.org/0day-ci/archive/20220424/202204242344.jepumdzp-...@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:165
virtio_crypto_alg_akcipher_init_session() error: potentially dereferencing
uninitialized 'input'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:230
virtio_crypto_alg_akcipher_close_session() error: uninitialized symbol
'vc_ctrl_req'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232
virtio_crypto_alg_akcipher_close_session() error: potentially dereferencing
uninitialized 'ctrl_status'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232
virtio_crypto_alg_akcipher_close_session() error: potentially dereferencing
uninitialized 'destroy_session'.
vim +/input +165 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
59ca6c93387d32 zhenwei pi 2022-03-02 99 static int
virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher_ctx *ctx,
59ca6c93387d32 zhenwei pi 2022-03-02 100 struct
virtio_crypto_ctrl_header *header, void *para,
59ca6c93387d32 zhenwei pi 2022-03-02 101 const uint8_t *key,
unsigned int keylen)
59ca6c93387d32 zhenwei pi 2022-03-02 102 {
59ca6c93387d32 zhenwei pi 2022-03-02 103 struct scatterlist outhdr_sg,
key_sg, inhdr_sg, *sgs[3];
59ca6c93387d32 zhenwei pi 2022-03-02 104 struct virtio_crypto *vcrypto =
ctx->vcrypto;
59ca6c93387d32 zhenwei pi 2022-03-02 105 uint8_t *pkey;
59ca6c93387d32 zhenwei pi 2022-03-02 106 unsigned int inlen;
59ca6c93387d32 zhenwei pi 2022-03-02 107 int err;
59ca6c93387d32 zhenwei pi 2022-03-02 108 unsigned int num_out = 0,
num_in = 0;
bb26cab9a7c25d zhenwei pi 2022-04-24 109 struct
virtio_crypto_op_ctrl_req *ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 110 struct
virtio_crypto_session_input *input;
286da9ed04239c zhenwei pi 2022-04-24 111 struct
virtio_crypto_ctrl_request *vc_ctrl_req;
59ca6c93387d32 zhenwei pi 2022-03-02 112
59ca6c93387d32 zhenwei pi 2022-03-02 113 pkey = kmemdup(key, keylen,
GFP_ATOMIC);
59ca6c93387d32 zhenwei pi 2022-03-02 114 if (!pkey)
59ca6c93387d32 zhenwei pi 2022-03-02 115 return -ENOMEM;
59ca6c93387d32 zhenwei pi 2022-03-02 116
286da9ed04239c zhenwei pi 2022-04-24 117 vc_ctrl_req =
kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
286da9ed04239c zhenwei pi 2022-04-24 118 if (!vc_ctrl_req) {
286da9ed04239c zhenwei pi 2022-04-24 119 err = -ENOMEM;
286da9ed04239c zhenwei pi 2022-04-24 120 goto out;
286da9ed04239c zhenwei pi 2022-04-24 121 }
286da9ed04239c zhenwei pi 2022-04-24 122
286da9ed04239c zhenwei pi 2022-04-24 123 ctrl = &vc_ctrl_req->ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 124 memcpy(&ctrl->header, header,
sizeof(ctrl->header));
bb26cab9a7c25d zhenwei pi 2022-04-24 125 memcpy(&ctrl->u, para,
sizeof(ctrl->u));
286da9ed04239c zhenwei pi 2022-04-24 126 input = &vc_ctrl_req->input;
bb26cab9a7c25d zhenwei pi 2022-04-24 127 input->status =
cpu_to_le32(VIRTIO_CRYPTO_ERR);
59ca6c93387d32 zhenwei pi 2022-03-02 128
bb26cab9a7c25d zhenwei pi 2022-04-24 129 sg_init_one(&outhdr_sg, ctrl,
sizeof(*ctrl));
59ca6c93387d32 zhenwei pi 2022-03-02 130 sgs[num_out++] = &outhdr_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 131
59ca6c93387d32 zhenwei pi 2022-03-02 132 sg_init_one(&key_sg, pkey,
keylen);
59ca6c93387d32 zhenwei pi 2022-03-02 133 sgs[num_out++] = &key_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 134
bb26cab9a7c25d zhenwei pi 2022-04-24 135 sg_init_one(&inhdr_sg, input,
sizeof(*input));
59ca6c93387d32 zhenwei pi 2022-03-02 136 sgs[num_out + num_in++] =
&inhdr_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 137
286da9ed04239c zhenwei pi 2022-04-24 138 spin_lock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 139 err =
virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
286da9ed04239c zhenwei pi 2022-04-24 140 if (err < 0) {
286da9ed04239c zhenwei pi 2022-04-24 141
spin_unlock(&vcrypto->ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 142 goto out;
286da9ed04239c zhenwei pi 2022-04-24 143 }
59ca6c93387d32 zhenwei pi 2022-03-02 144
59ca6c93387d32 zhenwei pi 2022-03-02 145
virtqueue_kick(vcrypto->ctrl_vq);
59ca6c93387d32 zhenwei pi 2022-03-02 146 while
(!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
59ca6c93387d32 zhenwei pi 2022-03-02 147
!virtqueue_
[PATCH v4 2/5] virtio-crypto: use private buffer for control request
Originally, all of the control requests share a single buffer(
ctrl & input & ctrl_status fields in struct virtio_crypto), this
allows queue depth 1 only, the performance of control queue gets
limited by this design.
In this patch, each request allocates request buffer dynamically, and
free buffer after request, so the scope protected by ctrl_lock also
get optimized here.
It's possible to optimize control queue depth in the next step.
A necessary comment is already in code, still describe it again:
/*
* Note: there are padding fields in request, clear them to zero before
* sending to host to avoid to divulge any information.
* Ex, virtio_crypto_ctrl_request::ctrl::u::destroy_session::padding[48]
*/
So use kzalloc to allocate buffer of struct virtio_crypto_ctrl_request.
Cc: Michael S. Tsirkin
Cc: Jason Wang
Cc: Gonglei
Signed-off-by: zhenwei pi
---
.../virtio/virtio_crypto_akcipher_algs.c | 41 +++
drivers/crypto/virtio/virtio_crypto_common.h | 17 +--
.../virtio/virtio_crypto_skcipher_algs.c | 50 ---
3 files changed, 75 insertions(+), 33 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index 20901a263fc8..509884e8b201 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -108,16 +108,22 @@ static int virtio_crypto_alg_akcipher_init_session(struct
virtio_crypto_akcipher
unsigned int num_out = 0, num_in = 0;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_session_input *input;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
pkey = kmemdup(key, keylen, GFP_ATOMIC);
if (!pkey)
return -ENOMEM;
- spin_lock(&vcrypto->ctrl_lock);
- ctrl = &vcrypto->ctrl;
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ ctrl = &vc_ctrl_req->ctrl;
memcpy(&ctrl->header, header, sizeof(ctrl->header));
memcpy(&ctrl->u, para, sizeof(ctrl->u));
- input = &vcrypto->input;
+ input = &vc_ctrl_req->input;
input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
sg_init_one(&outhdr_sg, ctrl, sizeof(*ctrl));
@@ -129,14 +135,18 @@ static int virtio_crypto_alg_akcipher_init_session(struct
virtio_crypto_akcipher
sg_init_one(&inhdr_sg, input, sizeof(*input));
sgs[num_out + num_in++] = &inhdr_sg;
+ spin_lock(&vcrypto->ctrl_lock);
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in,
vcrypto, GFP_ATOMIC);
- if (err < 0)
+ if (err < 0) {
+ spin_unlock(&vcrypto->ctrl_lock);
goto out;
+ }
virtqueue_kick(vcrypto->ctrl_vq);
while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
+ spin_unlock(&vcrypto->ctrl_lock);
if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
@@ -148,7 +158,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct
virtio_crypto_akcipher
err = 0;
out:
- spin_unlock(&vcrypto->ctrl_lock);
+ kfree(vc_ctrl_req);
kfree_sensitive(pkey);
if (err < 0)
@@ -167,15 +177,22 @@ static int
virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
int err;
struct virtio_crypto_op_ctrl_req *ctrl;
struct virtio_crypto_inhdr *ctrl_status;
+ struct virtio_crypto_ctrl_request *vc_ctrl_req;
- spin_lock(&vcrypto->ctrl_lock);
if (!ctx->session_valid) {
err = 0;
goto out;
}
- ctrl_status = &vcrypto->ctrl_status;
+
+ vc_ctrl_req = kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
+ if (!vc_ctrl_req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ ctrl_status = &vc_ctrl_req->ctrl_status;
ctrl_status->status = VIRTIO_CRYPTO_ERR;
- ctrl = &vcrypto->ctrl;
+ ctrl = &vc_ctrl_req->ctrl;
ctrl->header.opcode =
cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
ctrl->header.queue_id = 0;
@@ -188,14 +205,18 @@ static int
virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
sg_init_one(&inhdr_sg, &ctrl_status->status,
sizeof(ctrl_status->status));
sgs[num_out + num_in++] = &inhdr_sg;
+ spin_lock(&vcrypto->ctrl_lock);
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in,
vcrypto, GFP_ATOMIC);
- if (err < 0)
+ if (err < 0) {
+ spin_unlock(&vcrypto->ctrl_lock);
goto out;
+ }
virtqueue_kick(vcrypto->ctrl_vq);
while (!virtqueue_get_buf(vcrypto->ctrl_vq, &inlen) &&
!virtqueue_is_broken(vcrypto->ctrl_vq))
