Hi,
Currently the Linux virtio-crypto driver registers the crypto
algorithm without verifying if the backend actually supports the
algorithm.
This kernel patch series adds support for registering algorithm
with Linux crypto layer, only if the algorithm is supported by
the backend device. This also makes the driver more compliant with
the virtio-crypto spec [1].
I would appreciate any feedback or comments on this.
Thank you
Farhan
Reference
---------
[1] Virtio crypto spec proposal https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00816.html
Farhan Ali (2):
crypto/virtio-crypto: Read crypto services and algorithm masks
crypto/virtio-crypto: Register an algo only if it's supported
drivers/crypto/virtio/virtio_crypto_algs.c | 110 ++++++++++++++++++---------
drivers/crypto/virtio/virtio_crypto_common.h | 25 +++++-
drivers/crypto/virtio/virtio_crypto_core.c | 29 +++++++
drivers/crypto/virtio/virtio_crypto_mgr.c | 81 ++++++++++++++++++--
4 files changed, 201 insertions(+), 44 deletions(-)
--
2.7.4
From: Farhan Ali <[email protected]>
Register a crypto algo with the Linux crypto layer only if
the algorithm is supported by the backend virtio-crypto
device.
Also route crypto requests to a virtio-crypto
device, only if it can support the requested service and
algorithm.
Signed-off-by: Farhan Ali <[email protected]>
---
drivers/crypto/virtio/virtio_crypto_algs.c | 110 ++++++++++++++++++---------
drivers/crypto/virtio/virtio_crypto_common.h | 11 ++-
drivers/crypto/virtio/virtio_crypto_mgr.c | 81 ++++++++++++++++++--
3 files changed, 158 insertions(+), 44 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
index ba190cf..fef112a 100644
--- a/drivers/crypto/virtio/virtio_crypto_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_algs.c
@@ -49,12 +49,18 @@ struct virtio_crypto_sym_request {
bool encrypt;
};
+struct virtio_crypto_algo {
+ uint32_t algonum;
+ uint32_t service;
+ unsigned int active_devs;
+ struct crypto_alg algo;
+};
+
/*
* The algs_lock protects the below global virtio_crypto_active_devs
* and crypto algorithms registion.
*/
static DEFINE_MUTEX(algs_lock);
-static unsigned int virtio_crypto_active_devs;
static void virtio_crypto_ablkcipher_finalize_req(
struct virtio_crypto_sym_request *vc_sym_req,
struct ablkcipher_request *req,
@@ -312,13 +318,19 @@ static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm,
unsigned int keylen)
{
struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
+ uint32_t alg;
int ret;
+ ret = virtio_crypto_alg_validate_key(keylen, &alg);
+ if (ret)
+ return ret;
+
if (!ctx->vcrypto) {
/* New key */
int node = virtio_crypto_get_current_node();
struct virtio_crypto *vcrypto =
- virtcrypto_get_dev_node(node);
+ virtcrypto_get_dev_node(node,
+ VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
if (!vcrypto) {
pr_err("virtio_crypto: Could not find a virtio device in the system\n");
return -ENODEV;
@@ -571,57 +583,85 @@ static void virtio_crypto_ablkcipher_finalize_req(
virtcrypto_clear_request(&vc_sym_req->base);
}
-static struct crypto_alg virtio_crypto_algs[] = { {
- .cra_name = "cbc(aes)",
- .cra_driver_name = "virtio_crypto_aes_cbc",
- .cra_priority = 150,
- .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
- .cra_blocksize = AES_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct virtio_crypto_ablkcipher_ctx),
- .cra_alignmask = 0,
- .cra_module = THIS_MODULE,
- .cra_type = &crypto_ablkcipher_type,
- .cra_init = virtio_crypto_ablkcipher_init,
- .cra_exit = virtio_crypto_ablkcipher_exit,
- .cra_u = {
- .ablkcipher = {
- .setkey = virtio_crypto_ablkcipher_setkey,
- .decrypt = virtio_crypto_ablkcipher_decrypt,
- .encrypt = virtio_crypto_ablkcipher_encrypt,
- .min_keysize = AES_MIN_KEY_SIZE,
- .max_keysize = AES_MAX_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
+static struct virtio_crypto_algo virtio_crypto_algs[] = { {
+ .algonum = VIRTIO_CRYPTO_CIPHER_AES_CBC,
+ .service = VIRTIO_CRYPTO_SERVICE_CIPHER,
+ .algo = {
+ .cra_name = "cbc(aes)",
+ .cra_driver_name = "virtio_crypto_aes_cbc",
+ .cra_priority = 150,
+ .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
+ .cra_blocksize = AES_BLOCK_SIZE,
+ .cra_ctxsize = sizeof(struct virtio_crypto_ablkcipher_ctx),
+ .cra_alignmask = 0,
+ .cra_module = THIS_MODULE,
+ .cra_type = &crypto_ablkcipher_type,
+ .cra_init = virtio_crypto_ablkcipher_init,
+ .cra_exit = virtio_crypto_ablkcipher_exit,
+ .cra_u = {
+ .ablkcipher = {
+ .setkey = virtio_crypto_ablkcipher_setkey,
+ .decrypt = virtio_crypto_ablkcipher_decrypt,
+ .encrypt = virtio_crypto_ablkcipher_encrypt,
+ .min_keysize = AES_MIN_KEY_SIZE,
+ .max_keysize = AES_MAX_KEY_SIZE,
+ .ivsize = AES_BLOCK_SIZE,
+ },
},
},
} };
-int virtio_crypto_algs_register(void)
+int virtio_crypto_algs_register(struct virtio_crypto *vcrypto)
{
int ret = 0;
+ int i = 0;
mutex_lock(&algs_lock);
- if (++virtio_crypto_active_devs != 1)
- goto unlock;
- ret = crypto_register_algs(virtio_crypto_algs,
- ARRAY_SIZE(virtio_crypto_algs));
- if (ret)
- virtio_crypto_active_devs--;
+ for (i = 0; i < ARRAY_SIZE(virtio_crypto_algs); i++) {
+
+ uint32_t service = virtio_crypto_algs[i].service;
+ uint32_t algonum = virtio_crypto_algs[i].algonum;
+
+ if (!virtcrypto_algo_is_supported(vcrypto, service, algonum))
+ continue;
+
+ if (virtio_crypto_algs[i].active_devs == 0) {
+ ret = crypto_register_alg(&virtio_crypto_algs[i].algo);
+ if (ret)
+ goto unlock;
+ }
+
+ virtio_crypto_algs[i].active_devs++;
+ dev_info(&vcrypto->vdev->dev, "Registered algo %s\n",
+ virtio_crypto_algs[i].algo.cra_name);
+ }
unlock:
mutex_unlock(&algs_lock);
return ret;
}
-void virtio_crypto_algs_unregister(void)
+void virtio_crypto_algs_unregister(struct virtio_crypto *vcrypto)
{
+ int i = 0;
+
mutex_lock(&algs_lock);
- if (--virtio_crypto_active_devs != 0)
- goto unlock;
- crypto_unregister_algs(virtio_crypto_algs,
- ARRAY_SIZE(virtio_crypto_algs));
+ for (i = 0; i < ARRAY_SIZE(virtio_crypto_algs); i++) {
+
+ uint32_t service = virtio_crypto_algs[i].service;
+ uint32_t algonum = virtio_crypto_algs[i].algonum;
+
+ if (virtio_crypto_algs[i].active_devs == 0 ||
+ !virtcrypto_algo_is_supported(vcrypto, service, algonum))
+ continue;
+
+ if (virtio_crypto_algs[i].active_devs == 1)
+ crypto_unregister_alg(&virtio_crypto_algs[i].algo);
+
+ virtio_crypto_algs[i].active_devs--;
+ }
-unlock:
mutex_unlock(&algs_lock);
}
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index 05eca12e..76d49ec 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -116,7 +116,12 @@ int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev);
int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev);
void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev);
int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev);
-struct virtio_crypto *virtcrypto_get_dev_node(int node);
+bool virtcrypto_algo_is_supported(struct virtio_crypto *vcrypto_dev,
+ uint32_t service,
+ uint32_t algo);
+struct virtio_crypto *virtcrypto_get_dev_node(int node,
+ uint32_t service,
+ uint32_t algo);
int virtcrypto_dev_start(struct virtio_crypto *vcrypto);
void virtcrypto_dev_stop(struct virtio_crypto *vcrypto);
int virtio_crypto_ablkcipher_crypt_req(
@@ -136,7 +141,7 @@ static inline int virtio_crypto_get_current_node(void)
return node;
}
-int virtio_crypto_algs_register(void);
-void virtio_crypto_algs_unregister(void);
+int virtio_crypto_algs_register(struct virtio_crypto *vcrypto);
+void virtio_crypto_algs_unregister(struct virtio_crypto *vcrypto);
#endif /* _VIRTIO_CRYPTO_COMMON_H */
diff --git a/drivers/crypto/virtio/virtio_crypto_mgr.c b/drivers/crypto/virtio/virtio_crypto_mgr.c
index a69ff71..d70de3a 100644
--- a/drivers/crypto/virtio/virtio_crypto_mgr.c
+++ b/drivers/crypto/virtio/virtio_crypto_mgr.c
@@ -181,14 +181,20 @@ int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev)
/*
* virtcrypto_get_dev_node() - Get vcrypto_dev on the node.
* @node: Node id the driver works.
+ * @service: Crypto service that needs to be supported by the
+ * dev
+ * @algo: The algorithm number that needs to be supported by the
+ * dev
*
- * Function returns the virtio crypto device used fewest on the node.
+ * Function returns the virtio crypto device used fewest on the node,
+ * and supports the given crypto service and algorithm.
*
* To be used by virtio crypto device specific drivers.
*
* Return: pointer to vcrypto_dev or NULL if not found.
*/
-struct virtio_crypto *virtcrypto_get_dev_node(int node)
+struct virtio_crypto *virtcrypto_get_dev_node(int node, uint32_t service,
+ uint32_t algo)
{
struct virtio_crypto *vcrypto_dev = NULL, *tmp_dev;
unsigned long best = ~0;
@@ -199,7 +205,8 @@ struct virtio_crypto *virtcrypto_get_dev_node(int node)
if ((node == dev_to_node(&tmp_dev->vdev->dev) ||
dev_to_node(&tmp_dev->vdev->dev) < 0) &&
- virtcrypto_dev_started(tmp_dev)) {
+ virtcrypto_dev_started(tmp_dev) &&
+ virtcrypto_algo_is_supported(tmp_dev, service, algo)) {
ctr = atomic_read(&tmp_dev->ref_count);
if (best > ctr) {
vcrypto_dev = tmp_dev;
@@ -214,7 +221,9 @@ struct virtio_crypto *virtcrypto_get_dev_node(int node)
/* Get any started device */
list_for_each_entry(tmp_dev,
virtcrypto_devmgr_get_head(), list) {
- if (virtcrypto_dev_started(tmp_dev)) {
+ if (virtcrypto_dev_started(tmp_dev) &&
+ virtcrypto_algo_is_supported(tmp_dev,
+ service, algo)) {
vcrypto_dev = tmp_dev;
break;
}
@@ -240,7 +249,7 @@ struct virtio_crypto *virtcrypto_get_dev_node(int node)
*/
int virtcrypto_dev_start(struct virtio_crypto *vcrypto)
{
- if (virtio_crypto_algs_register()) {
+ if (virtio_crypto_algs_register(vcrypto)) {
pr_err("virtio_crypto: Failed to register crypto algs\n");
return -EFAULT;
}
@@ -260,5 +269,65 @@ int virtcrypto_dev_start(struct virtio_crypto *vcrypto)
*/
void virtcrypto_dev_stop(struct virtio_crypto *vcrypto)
{
- virtio_crypto_algs_unregister();
+ virtio_crypto_algs_unregister(vcrypto);
+}
+
+/*
+ * vcrypto_algo_is_supported()
+ * @vcrypto: Pointer to virtio crypto device.
+ * @service: The bit number for service validate.
+ * See VIRTIO_CRYPTO_SERVICE_*
+ * @algo : The bit number for the algorithm to validate.
+ *
+ *
+ * Validate if the virtio crypto device supports a service and
+ * algo.
+ *
+ * Return true if device supports a service and algo.
+ */
+
+bool virtcrypto_algo_is_supported(struct virtio_crypto *vcrypto,
+ uint32_t service,
+ uint32_t algo)
+{
+ uint32_t service_mask = 1u << service;
+ uint32_t algo_mask = 0;
+ bool low = true;
+
+ if (algo > 31) {
+ algo -= 32;
+ low = false;
+ }
+
+ if (!(vcrypto->crypto_services & service_mask))
+ return false;
+
+ switch (service) {
+ case VIRTIO_CRYPTO_SERVICE_CIPHER:
+ if (low)
+ algo_mask = vcrypto->cipher_algo_l;
+ else
+ algo_mask = vcrypto->cipher_algo_h;
+ break;
+
+ case VIRTIO_CRYPTO_SERVICE_HASH:
+ algo_mask = vcrypto->hash_algo;
+ break;
+
+ case VIRTIO_CRYPTO_SERVICE_MAC:
+ if (low)
+ algo_mask = vcrypto->mac_algo_l;
+ else
+ algo_mask = vcrypto->mac_algo_h;
+ break;
+
+ case VIRTIO_CRYPTO_SERVICE_AEAD:
+ algo_mask = vcrypto->aead_algo;
+ break;
+ }
+
+ if (!(algo_mask & (1u << algo)))
+ return false;
+
+ return true;
}
--
2.7.4
Read the crypto services and algorithm masks which provides
information about the services and algorithms supported by
virtio-crypto backend.
Signed-off-by: Farhan Ali <[email protected]>
---
drivers/crypto/virtio/virtio_crypto_common.h | 14 ++++++++++++++
drivers/crypto/virtio/virtio_crypto_core.c | 29 ++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index 66501a5..05eca12e 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -55,6 +55,20 @@ struct virtio_crypto {
/* Number of queue currently used by the driver */
u32 curr_queue;
+ /*
+ * Specifies the services mask which the device support,
+ * see VIRTIO_CRYPTO_SERVICE_* above
+ */
+ u32 crypto_services;
+
+ /* Detailed algorithms mask */
+ u32 cipher_algo_l;
+ u32 cipher_algo_h;
+ u32 hash_algo;
+ u32 mac_algo_l;
+ u32 mac_algo_h;
+ u32 aead_algo;
+
/* Maximum length of cipher key */
u32 max_cipher_key_len;
/* Maximum length of authenticated key */
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 8332698..8f745f2 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -303,6 +303,13 @@ static int virtcrypto_probe(struct virtio_device *vdev)
u32 max_data_queues = 0, max_cipher_key_len = 0;
u32 max_auth_key_len = 0;
u64 max_size = 0;
+ u32 cipher_algo_l = 0;
+ u32 cipher_algo_h = 0;
+ u32 hash_algo = 0;
+ u32 mac_algo_l = 0;
+ u32 mac_algo_h = 0;
+ u32 aead_algo = 0;
+ u32 crypto_services = 0;
if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
return -ENODEV;
@@ -339,6 +346,20 @@ static int virtcrypto_probe(struct virtio_device *vdev)
max_auth_key_len, &max_auth_key_len);
virtio_cread(vdev, struct virtio_crypto_config,
max_size, &max_size);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ crypto_services, &crypto_services);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ cipher_algo_l, &cipher_algo_l);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ cipher_algo_h, &cipher_algo_h);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ hash_algo, &hash_algo);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ mac_algo_l, &mac_algo_l);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ mac_algo_h, &mac_algo_h);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ aead_algo, &aead_algo);
/* Add virtio crypto device to global table */
err = virtcrypto_devmgr_add_dev(vcrypto);
@@ -358,6 +379,14 @@ static int virtcrypto_probe(struct virtio_device *vdev)
vcrypto->max_cipher_key_len = max_cipher_key_len;
vcrypto->max_auth_key_len = max_auth_key_len;
vcrypto->max_size = max_size;
+ vcrypto->crypto_services = crypto_services;
+ vcrypto->cipher_algo_l = cipher_algo_l;
+ vcrypto->cipher_algo_h = cipher_algo_h;
+ vcrypto->mac_algo_l = mac_algo_l;
+ vcrypto->mac_algo_h = mac_algo_h;
+ vcrypto->hash_algo = hash_algo;
+ vcrypto->aead_algo = aead_algo;
+
dev_info(&vdev->dev,
"max_queues: %u, max_cipher_key_len: %u, max_auth_key_len: %u, max_size 0x%llx\n",
--
2.7.4
> -----Original Message-----
> From: Farhan Ali [mailto:[email protected]]
> Sent: Saturday, June 09, 2018 3:09 AM
> To: [email protected]; [email protected]
> Cc: [email protected]; [email protected]; Gonglei (Arei)
> <[email protected]>; longpeng <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: [RFC v1 1/2] crypto/virtio-crypto: Read crypto services and algorithm
> masks
>
> Read the crypto services and algorithm masks which provides
> information about the services and algorithms supported by
> virtio-crypto backend.
>
> Signed-off-by: Farhan Ali <[email protected]>
> ---
> drivers/crypto/virtio/virtio_crypto_common.h | 14 ++++++++++++++
> drivers/crypto/virtio/virtio_crypto_core.c | 29
> ++++++++++++++++++++++++++++
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
> b/drivers/crypto/virtio/virtio_crypto_common.h
> index 66501a5..05eca12e 100644
> --- a/drivers/crypto/virtio/virtio_crypto_common.h
> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> @@ -55,6 +55,20 @@ struct virtio_crypto {
> /* Number of queue currently used by the driver */
> u32 curr_queue;
>
> + /*
> + * Specifies the services mask which the device support,
> + * see VIRTIO_CRYPTO_SERVICE_* above
> + */
Pls update the above comments. Except that:
Acked-by: Gonglei <[email protected]>
> + u32 crypto_services;
> +
> + /* Detailed algorithms mask */
> + u32 cipher_algo_l;
> + u32 cipher_algo_h;
> + u32 hash_algo;
> + u32 mac_algo_l;
> + u32 mac_algo_h;
> + u32 aead_algo;
> +
> /* Maximum length of cipher key */
> u32 max_cipher_key_len;
> /* Maximum length of authenticated key */
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
> b/drivers/crypto/virtio/virtio_crypto_core.c
> index 8332698..8f745f2 100644
> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -303,6 +303,13 @@ static int virtcrypto_probe(struct virtio_device *vdev)
> u32 max_data_queues = 0, max_cipher_key_len = 0;
> u32 max_auth_key_len = 0;
> u64 max_size = 0;
> + u32 cipher_algo_l = 0;
> + u32 cipher_algo_h = 0;
> + u32 hash_algo = 0;
> + u32 mac_algo_l = 0;
> + u32 mac_algo_h = 0;
> + u32 aead_algo = 0;
> + u32 crypto_services = 0;
>
> if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> return -ENODEV;
> @@ -339,6 +346,20 @@ static int virtcrypto_probe(struct virtio_device *vdev)
> max_auth_key_len, &max_auth_key_len);
> virtio_cread(vdev, struct virtio_crypto_config,
> max_size, &max_size);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + crypto_services, &crypto_services);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + cipher_algo_l, &cipher_algo_l);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + cipher_algo_h, &cipher_algo_h);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + hash_algo, &hash_algo);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + mac_algo_l, &mac_algo_l);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + mac_algo_h, &mac_algo_h);
> + virtio_cread(vdev, struct virtio_crypto_config,
> + aead_algo, &aead_algo);
>
> /* Add virtio crypto device to global table */
> err = virtcrypto_devmgr_add_dev(vcrypto);
> @@ -358,6 +379,14 @@ static int virtcrypto_probe(struct virtio_device *vdev)
> vcrypto->max_cipher_key_len = max_cipher_key_len;
> vcrypto->max_auth_key_len = max_auth_key_len;
> vcrypto->max_size = max_size;
> + vcrypto->crypto_services = crypto_services;
> + vcrypto->cipher_algo_l = cipher_algo_l;
> + vcrypto->cipher_algo_h = cipher_algo_h;
> + vcrypto->mac_algo_l = mac_algo_l;
> + vcrypto->mac_algo_h = mac_algo_h;
> + vcrypto->hash_algo = hash_algo;
> + vcrypto->aead_algo = aead_algo;
> +
>
> dev_info(&vdev->dev,
> "max_queues: %u, max_cipher_key_len: %u, max_auth_key_len: %u,
> max_size 0x%llx\n",
> --
> 2.7.4
> -----Original Message-----
> From: Farhan Ali [mailto:[email protected]]
> Sent: Saturday, June 09, 2018 3:09 AM
> To: [email protected]; [email protected]
> Cc: [email protected]; [email protected]; Gonglei (Arei)
> <[email protected]>; longpeng <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's supported
>
> From: Farhan Ali <[email protected]>
>
> Register a crypto algo with the Linux crypto layer only if
> the algorithm is supported by the backend virtio-crypto
> device.
>
> Also route crypto requests to a virtio-crypto
> device, only if it can support the requested service and
> algorithm.
>
> Signed-off-by: Farhan Ali <[email protected]>
> ---
> drivers/crypto/virtio/virtio_crypto_algs.c | 110
> ++++++++++++++++++---------
> drivers/crypto/virtio/virtio_crypto_common.h | 11 ++-
> drivers/crypto/virtio/virtio_crypto_mgr.c | 81 ++++++++++++++++++--
> 3 files changed, 158 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> b/drivers/crypto/virtio/virtio_crypto_algs.c
> index ba190cf..fef112a 100644
> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> @@ -49,12 +49,18 @@ struct virtio_crypto_sym_request {
> bool encrypt;
> };
>
> +struct virtio_crypto_algo {
> + uint32_t algonum;
> + uint32_t service;
> + unsigned int active_devs;
> + struct crypto_alg algo;
> +};
> +
> /*
> * The algs_lock protects the below global virtio_crypto_active_devs
> * and crypto algorithms registion.
> */
> static DEFINE_MUTEX(algs_lock);
> -static unsigned int virtio_crypto_active_devs;
> static void virtio_crypto_ablkcipher_finalize_req(
> struct virtio_crypto_sym_request *vc_sym_req,
> struct ablkcipher_request *req,
> @@ -312,13 +318,19 @@ static int virtio_crypto_ablkcipher_setkey(struct
> crypto_ablkcipher *tfm,
> unsigned int keylen)
> {
> struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
> + uint32_t alg;
> int ret;
>
> + ret = virtio_crypto_alg_validate_key(keylen, &alg);
> + if (ret)
> + return ret;
> +
> if (!ctx->vcrypto) {
> /* New key */
> int node = virtio_crypto_get_current_node();
> struct virtio_crypto *vcrypto =
> - virtcrypto_get_dev_node(node);
> + virtcrypto_get_dev_node(node,
> + VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
> if (!vcrypto) {
> pr_err("virtio_crypto: Could not find a virtio device in the
> system\n");
We'd better change the above error message now. What about:
" virtio_crypto: Could not find a virtio device in the system or unsupported algo" ?
Regards,
-Gonglei
Hi Arei
On 06/11/2018 02:43 AM, Gonglei (Arei) wrote:
>
>> -----Original Message-----
>> From: Farhan Ali [mailto:[email protected]]
>> Sent: Saturday, June 09, 2018 3:09 AM
>> To: [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; Gonglei (Arei)
>> <[email protected]>; longpeng <[email protected]>;
>> [email protected]; [email protected]; [email protected];
>> [email protected]
>> Subject: [RFC v1 1/2] crypto/virtio-crypto: Read crypto services and algorithm
>> masks
>>
>> Read the crypto services and algorithm masks which provides
>> information about the services and algorithms supported by
>> virtio-crypto backend.
>>
>> Signed-off-by: Farhan Ali <[email protected]>
>> ---
>> drivers/crypto/virtio/virtio_crypto_common.h | 14 ++++++++++++++
>> drivers/crypto/virtio/virtio_crypto_core.c | 29
>> ++++++++++++++++++++++++++++
>> 2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
>> b/drivers/crypto/virtio/virtio_crypto_common.h
>> index 66501a5..05eca12e 100644
>> --- a/drivers/crypto/virtio/virtio_crypto_common.h
>> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
>> @@ -55,6 +55,20 @@ struct virtio_crypto {
>> /* Number of queue currently used by the driver */
>> u32 curr_queue;
>>
>> + /*
>> + * Specifies the services mask which the device support,
>> + * see VIRTIO_CRYPTO_SERVICE_* above
>> + */
>
> Pls update the above comments. Except that:
>
> Acked-by: Gonglei <[email protected]>
>
Sure will update the comment. How about " Specifies the services mask
which the device support, * see VIRTIO_CRYPTO_SERVICE_*" ?
or should I specify the file where the VIRTIO_CRYPTO_SERVICE_* are defined?
Thanks
Farhan
>> + u32 crypto_services;
>> +
>> + /* Detailed algorithms mask */
>> + u32 cipher_algo_l;
>> + u32 cipher_algo_h;
>> + u32 hash_algo;
>> + u32 mac_algo_l;
>> + u32 mac_algo_h;
>> + u32 aead_algo;
>> +
>> /* Maximum length of cipher key */
>> u32 max_cipher_key_len;
>> /* Maximum length of authenticated key */
>> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
>> b/drivers/crypto/virtio/virtio_crypto_core.c
>> index 8332698..8f745f2 100644
>> --- a/drivers/crypto/virtio/virtio_crypto_core.c
>> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
>> @@ -303,6 +303,13 @@ static int virtcrypto_probe(struct virtio_device *vdev)
>> u32 max_data_queues = 0, max_cipher_key_len = 0;
>> u32 max_auth_key_len = 0;
>> u64 max_size = 0;
>> + u32 cipher_algo_l = 0;
>> + u32 cipher_algo_h = 0;
>> + u32 hash_algo = 0;
>> + u32 mac_algo_l = 0;
>> + u32 mac_algo_h = 0;
>> + u32 aead_algo = 0;
>> + u32 crypto_services = 0;
>>
>> if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>> return -ENODEV;
>> @@ -339,6 +346,20 @@ static int virtcrypto_probe(struct virtio_device *vdev)
>> max_auth_key_len, &max_auth_key_len);
>> virtio_cread(vdev, struct virtio_crypto_config,
>> max_size, &max_size);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + crypto_services, &crypto_services);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + cipher_algo_l, &cipher_algo_l);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + cipher_algo_h, &cipher_algo_h);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + hash_algo, &hash_algo);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + mac_algo_l, &mac_algo_l);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + mac_algo_h, &mac_algo_h);
>> + virtio_cread(vdev, struct virtio_crypto_config,
>> + aead_algo, &aead_algo);
>>
>> /* Add virtio crypto device to global table */
>> err = virtcrypto_devmgr_add_dev(vcrypto);
>> @@ -358,6 +379,14 @@ static int virtcrypto_probe(struct virtio_device *vdev)
>> vcrypto->max_cipher_key_len = max_cipher_key_len;
>> vcrypto->max_auth_key_len = max_auth_key_len;
>> vcrypto->max_size = max_size;
>> + vcrypto->crypto_services = crypto_services;
>> + vcrypto->cipher_algo_l = cipher_algo_l;
>> + vcrypto->cipher_algo_h = cipher_algo_h;
>> + vcrypto->mac_algo_l = mac_algo_l;
>> + vcrypto->mac_algo_h = mac_algo_h;
>> + vcrypto->hash_algo = hash_algo;
>> + vcrypto->aead_algo = aead_algo;
>> +
>>
>> dev_info(&vdev->dev,
>> "max_queues: %u, max_cipher_key_len: %u, max_auth_key_len: %u,
>> max_size 0x%llx\n",
>> --
>> 2.7.4
>
>
On 06/11/2018 04:48 AM, Gonglei (Arei) wrote:
>
>
>> -----Original Message-----
>> From: Farhan Ali [mailto:[email protected]]
>> Sent: Saturday, June 09, 2018 3:09 AM
>> To: [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; Gonglei (Arei)
>> <[email protected]>; longpeng <[email protected]>;
>> [email protected]; [email protected]; [email protected];
>> [email protected]
>> Subject: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's supported
>>
>> From: Farhan Ali <[email protected]>
>>
>> Register a crypto algo with the Linux crypto layer only if
>> the algorithm is supported by the backend virtio-crypto
>> device.
>>
>> Also route crypto requests to a virtio-crypto
>> device, only if it can support the requested service and
>> algorithm.
>>
>> Signed-off-by: Farhan Ali <[email protected]>
>> ---
>> drivers/crypto/virtio/virtio_crypto_algs.c | 110
>> ++++++++++++++++++---------
>> drivers/crypto/virtio/virtio_crypto_common.h | 11 ++-
>> drivers/crypto/virtio/virtio_crypto_mgr.c | 81 ++++++++++++++++++--
>> 3 files changed, 158 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
>> b/drivers/crypto/virtio/virtio_crypto_algs.c
>> index ba190cf..fef112a 100644
>> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
>> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
>> @@ -49,12 +49,18 @@ struct virtio_crypto_sym_request {
>> bool encrypt;
>> };
>>
>> +struct virtio_crypto_algo {
>> + uint32_t algonum;
>> + uint32_t service;
>> + unsigned int active_devs;
>> + struct crypto_alg algo;
>> +};
>> +
>> /*
>> * The algs_lock protects the below global virtio_crypto_active_devs
>> * and crypto algorithms registion.
>> */
>> static DEFINE_MUTEX(algs_lock);
>> -static unsigned int virtio_crypto_active_devs;
>> static void virtio_crypto_ablkcipher_finalize_req(
>> struct virtio_crypto_sym_request *vc_sym_req,
>> struct ablkcipher_request *req,
>> @@ -312,13 +318,19 @@ static int virtio_crypto_ablkcipher_setkey(struct
>> crypto_ablkcipher *tfm,
>> unsigned int keylen)
>> {
>> struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
>> + uint32_t alg;
>> int ret;
>>
>> + ret = virtio_crypto_alg_validate_key(keylen, &alg);
>> + if (ret)
>> + return ret;
>> +
>> if (!ctx->vcrypto) {
>> /* New key */
>> int node = virtio_crypto_get_current_node();
>> struct virtio_crypto *vcrypto =
>> - virtcrypto_get_dev_node(node);
>> + virtcrypto_get_dev_node(node,
>> + VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
>> if (!vcrypto) {
>> pr_err("virtio_crypto: Could not find a virtio device in the
>> system\n");
>
> We'd better change the above error message now. What about:
> " virtio_crypto: Could not find a virtio device in the system or unsupported algo" ?
>
> Regards,
> -Gonglei
Sure, I will update the error message. But other than that does the rest
of the code looks good to you?
Thanks
Farhan
>
>
>
>
> -----Original Message-----
> From: Farhan Ali [mailto:[email protected]]
> Sent: Wednesday, June 13, 2018 1:07 AM
> To: Gonglei (Arei) <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; longpeng
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC v1 1/2] crypto/virtio-crypto: Read crypto services and
> algorithm masks
>
> Hi Arei
>
> On 06/11/2018 02:43 AM, Gonglei (Arei) wrote:
> >
> >> -----Original Message-----
> >> From: Farhan Ali [mailto:[email protected]]
> >> Sent: Saturday, June 09, 2018 3:09 AM
> >> To: [email protected]; [email protected]
> >> Cc: [email protected]; [email protected]; Gonglei (Arei)
> >> <[email protected]>; longpeng <[email protected]>;
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]
> >> Subject: [RFC v1 1/2] crypto/virtio-crypto: Read crypto services and
> algorithm
> >> masks
> >>
> >> Read the crypto services and algorithm masks which provides
> >> information about the services and algorithms supported by
> >> virtio-crypto backend.
> >>
> >> Signed-off-by: Farhan Ali <[email protected]>
> >> ---
> >> drivers/crypto/virtio/virtio_crypto_common.h | 14 ++++++++++++++
> >> drivers/crypto/virtio/virtio_crypto_core.c | 29
> >> ++++++++++++++++++++++++++++
> >> 2 files changed, 43 insertions(+)
> >>
> >> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
> >> b/drivers/crypto/virtio/virtio_crypto_common.h
> >> index 66501a5..05eca12e 100644
> >> --- a/drivers/crypto/virtio/virtio_crypto_common.h
> >> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> >> @@ -55,6 +55,20 @@ struct virtio_crypto {
> >> /* Number of queue currently used by the driver */
> >> u32 curr_queue;
> >>
> >> + /*
> >> + * Specifies the services mask which the device support,
> >> + * see VIRTIO_CRYPTO_SERVICE_* above
> >> + */
> >
> > Pls update the above comments. Except that:
> >
> > Acked-by: Gonglei <[email protected]>
> >
>
> Sure will update the comment. How about " Specifies the services mask
> which the device support, * see VIRTIO_CRYPTO_SERVICE_*" ?
>
It makes sense IMHO :)
Regards,
-Gonglei
> or should I specify the file where the VIRTIO_CRYPTO_SERVICE_* are defined?
>
> Thanks
> Farhan
>
> >> + u32 crypto_services;
> >> +
> >> + /* Detailed algorithms mask */
> >> + u32 cipher_algo_l;
> >> + u32 cipher_algo_h;
> >> + u32 hash_algo;
> >> + u32 mac_algo_l;
> >> + u32 mac_algo_h;
> >> + u32 aead_algo;
> >> +
> >> /* Maximum length of cipher key */
> >> u32 max_cipher_key_len;
> >> /* Maximum length of authenticated key */
> >> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
> >> b/drivers/crypto/virtio/virtio_crypto_core.c
> >> index 8332698..8f745f2 100644
> >> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> >> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> >> @@ -303,6 +303,13 @@ static int virtcrypto_probe(struct virtio_device
> *vdev)
> >> u32 max_data_queues = 0, max_cipher_key_len = 0;
> >> u32 max_auth_key_len = 0;
> >> u64 max_size = 0;
> >> + u32 cipher_algo_l = 0;
> >> + u32 cipher_algo_h = 0;
> >> + u32 hash_algo = 0;
> >> + u32 mac_algo_l = 0;
> >> + u32 mac_algo_h = 0;
> >> + u32 aead_algo = 0;
> >> + u32 crypto_services = 0;
> >>
> >> if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> >> return -ENODEV;
> >> @@ -339,6 +346,20 @@ static int virtcrypto_probe(struct virtio_device
> *vdev)
> >> max_auth_key_len, &max_auth_key_len);
> >> virtio_cread(vdev, struct virtio_crypto_config,
> >> max_size, &max_size);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + crypto_services, &crypto_services);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + cipher_algo_l, &cipher_algo_l);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + cipher_algo_h, &cipher_algo_h);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + hash_algo, &hash_algo);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + mac_algo_l, &mac_algo_l);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + mac_algo_h, &mac_algo_h);
> >> + virtio_cread(vdev, struct virtio_crypto_config,
> >> + aead_algo, &aead_algo);
> >>
> >> /* Add virtio crypto device to global table */
> >> err = virtcrypto_devmgr_add_dev(vcrypto);
> >> @@ -358,6 +379,14 @@ static int virtcrypto_probe(struct virtio_device
> *vdev)
> >> vcrypto->max_cipher_key_len = max_cipher_key_len;
> >> vcrypto->max_auth_key_len = max_auth_key_len;
> >> vcrypto->max_size = max_size;
> >> + vcrypto->crypto_services = crypto_services;
> >> + vcrypto->cipher_algo_l = cipher_algo_l;
> >> + vcrypto->cipher_algo_h = cipher_algo_h;
> >> + vcrypto->mac_algo_l = mac_algo_l;
> >> + vcrypto->mac_algo_h = mac_algo_h;
> >> + vcrypto->hash_algo = hash_algo;
> >> + vcrypto->aead_algo = aead_algo;
> >> +
> >>
> >> dev_info(&vdev->dev,
> >> "max_queues: %u, max_cipher_key_len: %u,
> max_auth_key_len: %u,
> >> max_size 0x%llx\n",
> >> --
> >> 2.7.4
> >
> >
> -----Original Message-----
> From: Farhan Ali [mailto:[email protected]]
> Sent: Wednesday, June 13, 2018 1:08 AM
> To: Gonglei (Arei) <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]; longpeng
> <[email protected]>; [email protected]; [email protected];
> [email protected]
> Subject: Re: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's
> supported
>
>
>
> On 06/11/2018 04:48 AM, Gonglei (Arei) wrote:
> >
> >
> >> -----Original Message-----
> >> From: Farhan Ali [mailto:[email protected]]
> >> Sent: Saturday, June 09, 2018 3:09 AM
> >> To: [email protected]; [email protected]
> >> Cc: [email protected]; [email protected]; Gonglei (Arei)
> >> <[email protected]>; longpeng <[email protected]>;
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]
> >> Subject: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's
> supported
> >>
> >> From: Farhan Ali <[email protected]>
> >>
> >> Register a crypto algo with the Linux crypto layer only if
> >> the algorithm is supported by the backend virtio-crypto
> >> device.
> >>
> >> Also route crypto requests to a virtio-crypto
> >> device, only if it can support the requested service and
> >> algorithm.
> >>
> >> Signed-off-by: Farhan Ali <[email protected]>
> >> ---
> >> drivers/crypto/virtio/virtio_crypto_algs.c | 110
> >> ++++++++++++++++++---------
> >> drivers/crypto/virtio/virtio_crypto_common.h | 11 ++-
> >> drivers/crypto/virtio/virtio_crypto_mgr.c | 81
> ++++++++++++++++++--
> >> 3 files changed, 158 insertions(+), 44 deletions(-)
> >>
> >> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> >> b/drivers/crypto/virtio/virtio_crypto_algs.c
> >> index ba190cf..fef112a 100644
> >> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
> >> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> >> @@ -49,12 +49,18 @@ struct virtio_crypto_sym_request {
> >> bool encrypt;
> >> };
> >>
> >> +struct virtio_crypto_algo {
> >> + uint32_t algonum;
> >> + uint32_t service;
> >> + unsigned int active_devs;
> >> + struct crypto_alg algo;
> >> +};
> >> +
> >> /*
> >> * The algs_lock protects the below global virtio_crypto_active_devs
> >> * and crypto algorithms registion.
> >> */
> >> static DEFINE_MUTEX(algs_lock);
> >> -static unsigned int virtio_crypto_active_devs;
> >> static void virtio_crypto_ablkcipher_finalize_req(
> >> struct virtio_crypto_sym_request *vc_sym_req,
> >> struct ablkcipher_request *req,
> >> @@ -312,13 +318,19 @@ static int virtio_crypto_ablkcipher_setkey(struct
> >> crypto_ablkcipher *tfm,
> >> unsigned int keylen)
> >> {
> >> struct virtio_crypto_ablkcipher_ctx *ctx =
> crypto_ablkcipher_ctx(tfm);
> >> + uint32_t alg;
> >> int ret;
> >>
> >> + ret = virtio_crypto_alg_validate_key(keylen, &alg);
> >> + if (ret)
> >> + return ret;
> >> +
> >> if (!ctx->vcrypto) {
> >> /* New key */
> >> int node = virtio_crypto_get_current_node();
> >> struct virtio_crypto *vcrypto =
> >> - virtcrypto_get_dev_node(node);
> >> + virtcrypto_get_dev_node(node,
> >> + VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
> >> if (!vcrypto) {
> >> pr_err("virtio_crypto: Could not find a virtio device in the
> >> system\n");
> >
> > We'd better change the above error message now. What about:
> > " virtio_crypto: Could not find a virtio device in the system or unsupported
> algo" ?
> >
> > Regards,
> > -Gonglei
>
>
> Sure, I will update the error message. But other than that does the rest
> of the code looks good to you?
>
Yes, good work. You can add my ack in v2:
Acked-by: Gonglei <[email protected]>
Regards,
-Gonglei