* [PATCH 0/5] crypto: crypto_user_stat: misc enhancement
@ 2018-11-05 12:51 Corentin Labbe
2018-11-05 12:51 ` [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional Corentin Labbe
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Corentin Labbe @ 2018-11-05 12:51 UTC (permalink / raw)
To: davem, ebiggers, herbert; +Cc: linux-crypto, linux-kernel, Corentin Labbe
Hello
This patchset fixes all reported problem by Eric biggers.
This patchset should be applied on top of Eric biggers patchset "crypto: crypto_user reporting fixes and cleanups"
Regards
Corentin Labbe (5):
crypto: crypto_user_stat: made crypto_user_stat optional
crypto: crypto_user_stat: convert all stats from u32 to u64
crypto: crypto_user_stat: split user space crypto stat structures
crypto: tool: getstat: convert user space example to the new
crypto_user_stat uapi
crypto: crypto_user_stat: fix use_after_free of struct xxx_request
crypto/Makefile | 3 +-
crypto/ahash.c | 14 ++-
crypto/algapi.c | 12 ++-
crypto/crypto_user_stat.c | 134 +++++++++++++--------------
include/crypto/acompress.h | 30 +++---
include/crypto/aead.h | 30 +++---
include/crypto/akcipher.h | 56 ++++++-----
include/crypto/hash.h | 25 ++---
include/crypto/internal/cryptouser.h | 17 ++++
include/crypto/kpp.h | 26 +++---
include/crypto/rng.h | 8 +-
include/crypto/skcipher.h | 24 +++--
include/linux/crypto.h | 74 +++++++--------
include/uapi/linux/cryptouser.h | 103 ++++++++++++--------
tools/crypto/getstat.c | 54 +++++------
15 files changed, 329 insertions(+), 281 deletions(-)
--
2.18.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional
2018-11-05 12:51 [PATCH 0/5] crypto: crypto_user_stat: misc enhancement Corentin Labbe
@ 2018-11-05 12:51 ` Corentin Labbe
2018-11-06 1:41 ` Eric Biggers
2018-11-05 12:51 ` [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64 Corentin Labbe
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Corentin Labbe @ 2018-11-05 12:51 UTC (permalink / raw)
To: davem, ebiggers, herbert; +Cc: linux-crypto, linux-kernel, Corentin Labbe
Even if CRYPTO_STATS is set to n, some part of CRYPTO_STATS are
compiled.
This patch made all part of crypto_user_stat uncompiled in that case.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
crypto/Makefile | 3 ++-
crypto/algapi.c | 2 ++
include/crypto/internal/cryptouser.h | 17 +++++++++++++++++
include/linux/crypto.h | 2 ++
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/crypto/Makefile b/crypto/Makefile
index 5c207c76abf7..1e9e5960946a 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -54,7 +54,8 @@ cryptomgr-y := algboss.o testmgr.o
obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
obj-$(CONFIG_CRYPTO_USER) += crypto_user.o
-crypto_user-y := crypto_user_base.o crypto_user_stat.o
+crypto_user-y := crypto_user_base.o
+crypto_user-$(CONFIG_CRYPTO_STATS) += crypto_user_stat.o
obj-$(CONFIG_CRYPTO_CMAC) += cmac.o
obj-$(CONFIG_CRYPTO_HMAC) += hmac.o
obj-$(CONFIG_CRYPTO_VMAC) += vmac.o
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 2545c5f89c4c..f5396c88e8cd 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -258,6 +258,7 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
list_add(&alg->cra_list, &crypto_alg_list);
list_add(&larval->alg.cra_list, &crypto_alg_list);
+#ifdef CONFIG_CRYPTO_STATS
atomic_set(&alg->encrypt_cnt, 0);
atomic_set(&alg->decrypt_cnt, 0);
atomic64_set(&alg->encrypt_tlen, 0);
@@ -265,6 +266,7 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
atomic_set(&alg->verify_cnt, 0);
atomic_set(&alg->cipher_err_cnt, 0);
atomic_set(&alg->sign_cnt, 0);
+#endif
out:
return larval;
diff --git a/include/crypto/internal/cryptouser.h b/include/crypto/internal/cryptouser.h
index 8db299c25566..3492ab42eefb 100644
--- a/include/crypto/internal/cryptouser.h
+++ b/include/crypto/internal/cryptouser.h
@@ -3,6 +3,23 @@
struct crypto_alg *crypto_alg_match(struct crypto_user_alg *p, int exact);
+#ifdef CONFIG_CRYPTO_STATS
int crypto_dump_reportstat(struct sk_buff *skb, struct netlink_callback *cb);
int crypto_reportstat(struct sk_buff *in_skb, struct nlmsghdr *in_nlh, struct nlattr **attrs);
int crypto_dump_reportstat_done(struct netlink_callback *cb);
+#else
+static int crypto_dump_reportstat(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ return -ENOTSUPP;
+}
+
+static int crypto_reportstat(struct sk_buff *in_skb, struct nlmsghdr *in_nlh, struct nlattr **attrs)
+{
+ return -ENOTSUPP;
+}
+
+static int crypto_dump_reportstat_done(struct netlink_callback *cb)
+{
+ return -ENOTSUPP;
+}
+#endif
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 3634ad6fe202..35de61d99cd5 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -515,6 +515,7 @@ struct crypto_alg {
struct module *cra_module;
+#ifdef CONFIG_CRYPTO_STATS
union {
atomic_t encrypt_cnt;
atomic_t compress_cnt;
@@ -552,6 +553,7 @@ struct crypto_alg {
atomic_t compute_shared_secret_cnt;
};
atomic_t sign_cnt;
+#endif
} CRYPTO_MINALIGN_ATTR;
--
2.18.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64
2018-11-05 12:51 [PATCH 0/5] crypto: crypto_user_stat: misc enhancement Corentin Labbe
2018-11-05 12:51 ` [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional Corentin Labbe
@ 2018-11-05 12:51 ` Corentin Labbe
2018-11-06 1:42 ` Eric Biggers
2018-11-05 12:51 ` [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures Corentin Labbe
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Corentin Labbe @ 2018-11-05 12:51 UTC (permalink / raw)
To: davem, ebiggers, herbert; +Cc: linux-crypto, linux-kernel, Corentin Labbe
All the 32-bit fields need to be 64-bit. In some cases, UINT32_MAX crypto
operations can be done in seconds.
Reported-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
crypto/algapi.c | 10 +--
crypto/crypto_user_stat.c | 114 +++++++++++++++-----------------
include/crypto/acompress.h | 8 +--
include/crypto/aead.h | 8 +--
include/crypto/akcipher.h | 16 ++---
include/crypto/hash.h | 6 +-
include/crypto/kpp.h | 12 ++--
include/crypto/rng.h | 8 +--
include/crypto/skcipher.h | 8 +--
include/linux/crypto.h | 46 ++++++-------
include/uapi/linux/cryptouser.h | 38 +++++------
11 files changed, 133 insertions(+), 141 deletions(-)
diff --git a/crypto/algapi.c b/crypto/algapi.c
index f5396c88e8cd..42fe316f80ee 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -259,13 +259,13 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
list_add(&larval->alg.cra_list, &crypto_alg_list);
#ifdef CONFIG_CRYPTO_STATS
- atomic_set(&alg->encrypt_cnt, 0);
- atomic_set(&alg->decrypt_cnt, 0);
+ atomic64_set(&alg->encrypt_cnt, 0);
+ atomic64_set(&alg->decrypt_cnt, 0);
atomic64_set(&alg->encrypt_tlen, 0);
atomic64_set(&alg->decrypt_tlen, 0);
- atomic_set(&alg->verify_cnt, 0);
- atomic_set(&alg->cipher_err_cnt, 0);
- atomic_set(&alg->sign_cnt, 0);
+ atomic64_set(&alg->verify_cnt, 0);
+ atomic64_set(&alg->cipher_err_cnt, 0);
+ atomic64_set(&alg->sign_cnt, 0);
#endif
out:
diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
index a6fb2e6f618d..352569f378a0 100644
--- a/crypto/crypto_user_stat.c
+++ b/crypto/crypto_user_stat.c
@@ -35,22 +35,21 @@ static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat raead;
u64 v64;
- u32 v32;
memset(&raead, 0, sizeof(raead));
strscpy(raead.type, "aead", sizeof(raead.type));
- v32 = atomic_read(&alg->encrypt_cnt);
- raead.stat_encrypt_cnt = v32;
+ v64 = atomic64_read(&alg->encrypt_cnt);
+ raead.stat_encrypt_cnt = v64;
v64 = atomic64_read(&alg->encrypt_tlen);
raead.stat_encrypt_tlen = v64;
- v32 = atomic_read(&alg->decrypt_cnt);
- raead.stat_decrypt_cnt = v32;
+ v64 = atomic64_read(&alg->decrypt_cnt);
+ raead.stat_decrypt_cnt = v64;
v64 = atomic64_read(&alg->decrypt_tlen);
raead.stat_decrypt_tlen = v64;
- v32 = atomic_read(&alg->aead_err_cnt);
- raead.stat_aead_err_cnt = v32;
+ v64 = atomic64_read(&alg->aead_err_cnt);
+ raead.stat_aead_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_AEAD, sizeof(raead), &raead);
}
@@ -59,22 +58,21 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rcipher;
u64 v64;
- u32 v32;
memset(&rcipher, 0, sizeof(rcipher));
strscpy(rcipher.type, "cipher", sizeof(rcipher.type));
- v32 = atomic_read(&alg->encrypt_cnt);
- rcipher.stat_encrypt_cnt = v32;
+ v64 = atomic64_read(&alg->encrypt_cnt);
+ rcipher.stat_encrypt_cnt = v64;
v64 = atomic64_read(&alg->encrypt_tlen);
rcipher.stat_encrypt_tlen = v64;
- v32 = atomic_read(&alg->decrypt_cnt);
- rcipher.stat_decrypt_cnt = v32;
+ v64 = atomic64_read(&alg->decrypt_cnt);
+ rcipher.stat_decrypt_cnt = v64;
v64 = atomic64_read(&alg->decrypt_tlen);
rcipher.stat_decrypt_tlen = v64;
- v32 = atomic_read(&alg->cipher_err_cnt);
- rcipher.stat_cipher_err_cnt = v32;
+ v64 = atomic64_read(&alg->cipher_err_cnt);
+ rcipher.stat_cipher_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_CIPHER, sizeof(rcipher), &rcipher);
}
@@ -83,21 +81,20 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rcomp;
u64 v64;
- u32 v32;
memset(&rcomp, 0, sizeof(rcomp));
strscpy(rcomp.type, "compression", sizeof(rcomp.type));
- v32 = atomic_read(&alg->compress_cnt);
- rcomp.stat_compress_cnt = v32;
+ v64 = atomic64_read(&alg->compress_cnt);
+ rcomp.stat_compress_cnt = v64;
v64 = atomic64_read(&alg->compress_tlen);
rcomp.stat_compress_tlen = v64;
- v32 = atomic_read(&alg->decompress_cnt);
- rcomp.stat_decompress_cnt = v32;
+ v64 = atomic64_read(&alg->decompress_cnt);
+ rcomp.stat_decompress_cnt = v64;
v64 = atomic64_read(&alg->decompress_tlen);
rcomp.stat_decompress_tlen = v64;
- v32 = atomic_read(&alg->cipher_err_cnt);
- rcomp.stat_compress_err_cnt = v32;
+ v64 = atomic64_read(&alg->cipher_err_cnt);
+ rcomp.stat_compress_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_COMPRESS, sizeof(rcomp), &rcomp);
}
@@ -106,21 +103,20 @@ static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat racomp;
u64 v64;
- u32 v32;
memset(&racomp, 0, sizeof(racomp));
strscpy(racomp.type, "acomp", sizeof(racomp.type));
- v32 = atomic_read(&alg->compress_cnt);
- racomp.stat_compress_cnt = v32;
+ v64 = atomic64_read(&alg->compress_cnt);
+ racomp.stat_compress_cnt = v64;
v64 = atomic64_read(&alg->compress_tlen);
racomp.stat_compress_tlen = v64;
- v32 = atomic_read(&alg->decompress_cnt);
- racomp.stat_decompress_cnt = v32;
+ v64 = atomic64_read(&alg->decompress_cnt);
+ racomp.stat_decompress_cnt = v64;
v64 = atomic64_read(&alg->decompress_tlen);
racomp.stat_decompress_tlen = v64;
- v32 = atomic_read(&alg->cipher_err_cnt);
- racomp.stat_compress_err_cnt = v32;
+ v64 = atomic64_read(&alg->cipher_err_cnt);
+ racomp.stat_compress_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_ACOMP, sizeof(racomp), &racomp);
}
@@ -129,25 +125,24 @@ static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rakcipher;
u64 v64;
- u32 v32;
memset(&rakcipher, 0, sizeof(rakcipher));
strscpy(rakcipher.type, "akcipher", sizeof(rakcipher.type));
- v32 = atomic_read(&alg->encrypt_cnt);
- rakcipher.stat_encrypt_cnt = v32;
+ v64 = atomic64_read(&alg->encrypt_cnt);
+ rakcipher.stat_encrypt_cnt = v64;
v64 = atomic64_read(&alg->encrypt_tlen);
rakcipher.stat_encrypt_tlen = v64;
- v32 = atomic_read(&alg->decrypt_cnt);
- rakcipher.stat_decrypt_cnt = v32;
+ v64 = atomic64_read(&alg->decrypt_cnt);
+ rakcipher.stat_decrypt_cnt = v64;
v64 = atomic64_read(&alg->decrypt_tlen);
rakcipher.stat_decrypt_tlen = v64;
- v32 = atomic_read(&alg->sign_cnt);
- rakcipher.stat_sign_cnt = v32;
- v32 = atomic_read(&alg->verify_cnt);
- rakcipher.stat_verify_cnt = v32;
- v32 = atomic_read(&alg->akcipher_err_cnt);
- rakcipher.stat_akcipher_err_cnt = v32;
+ v64 = atomic64_read(&alg->sign_cnt);
+ rakcipher.stat_sign_cnt = v64;
+ v64 = atomic64_read(&alg->verify_cnt);
+ rakcipher.stat_verify_cnt = v64;
+ v64 = atomic64_read(&alg->akcipher_err_cnt);
+ rakcipher.stat_akcipher_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_AKCIPHER,
sizeof(rakcipher), &rakcipher);
@@ -156,19 +151,19 @@ static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rkpp;
- u32 v;
+ u64 v;
memset(&rkpp, 0, sizeof(rkpp));
strscpy(rkpp.type, "kpp", sizeof(rkpp.type));
- v = atomic_read(&alg->setsecret_cnt);
+ v = atomic64_read(&alg->setsecret_cnt);
rkpp.stat_setsecret_cnt = v;
- v = atomic_read(&alg->generate_public_key_cnt);
+ v = atomic64_read(&alg->generate_public_key_cnt);
rkpp.stat_generate_public_key_cnt = v;
- v = atomic_read(&alg->compute_shared_secret_cnt);
+ v = atomic64_read(&alg->compute_shared_secret_cnt);
rkpp.stat_compute_shared_secret_cnt = v;
- v = atomic_read(&alg->kpp_err_cnt);
+ v = atomic64_read(&alg->kpp_err_cnt);
rkpp.stat_kpp_err_cnt = v;
return nla_put(skb, CRYPTOCFGA_STAT_KPP, sizeof(rkpp), &rkpp);
@@ -178,18 +173,17 @@ static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rhash;
u64 v64;
- u32 v32;
memset(&rhash, 0, sizeof(rhash));
strscpy(rhash.type, "ahash", sizeof(rhash.type));
- v32 = atomic_read(&alg->hash_cnt);
- rhash.stat_hash_cnt = v32;
+ v64 = atomic64_read(&alg->hash_cnt);
+ rhash.stat_hash_cnt = v64;
v64 = atomic64_read(&alg->hash_tlen);
rhash.stat_hash_tlen = v64;
- v32 = atomic_read(&alg->hash_err_cnt);
- rhash.stat_hash_err_cnt = v32;
+ v64 = atomic64_read(&alg->hash_err_cnt);
+ rhash.stat_hash_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_HASH, sizeof(rhash), &rhash);
}
@@ -198,18 +192,17 @@ static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rhash;
u64 v64;
- u32 v32;
memset(&rhash, 0, sizeof(rhash));
strscpy(rhash.type, "shash", sizeof(rhash.type));
- v32 = atomic_read(&alg->hash_cnt);
- rhash.stat_hash_cnt = v32;
+ v64 = atomic64_read(&alg->hash_cnt);
+ rhash.stat_hash_cnt = v64;
v64 = atomic64_read(&alg->hash_tlen);
rhash.stat_hash_tlen = v64;
- v32 = atomic_read(&alg->hash_err_cnt);
- rhash.stat_hash_err_cnt = v32;
+ v64 = atomic64_read(&alg->hash_err_cnt);
+ rhash.stat_hash_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_HASH, sizeof(rhash), &rhash);
}
@@ -218,20 +211,19 @@ static int crypto_report_rng(struct sk_buff *skb, struct crypto_alg *alg)
{
struct crypto_stat rrng;
u64 v64;
- u32 v32;
memset(&rrng, 0, sizeof(rrng));
strscpy(rrng.type, "rng", sizeof(rrng.type));
- v32 = atomic_read(&alg->generate_cnt);
- rrng.stat_generate_cnt = v32;
+ v64 = atomic64_read(&alg->generate_cnt);
+ rrng.stat_generate_cnt = v64;
v64 = atomic64_read(&alg->generate_tlen);
rrng.stat_generate_tlen = v64;
- v32 = atomic_read(&alg->seed_cnt);
- rrng.stat_seed_cnt = v32;
- v32 = atomic_read(&alg->hash_err_cnt);
- rrng.stat_rng_err_cnt = v32;
+ v64 = atomic64_read(&alg->seed_cnt);
+ rrng.stat_seed_cnt = v64;
+ v64 = atomic64_read(&alg->hash_err_cnt);
+ rrng.stat_rng_err_cnt = v64;
return nla_put(skb, CRYPTOCFGA_STAT_RNG, sizeof(rrng), &rrng);
}
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index 22e6f412c595..f79918196811 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -240,9 +240,9 @@ static inline void crypto_stat_compress(struct acomp_req *req, int ret)
struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->compress_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->compress_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->compress_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->compress_cnt);
atomic64_add(req->slen, &tfm->base.__crt_alg->compress_tlen);
}
#endif
@@ -254,9 +254,9 @@ static inline void crypto_stat_decompress(struct acomp_req *req, int ret)
struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->compress_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->compress_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->decompress_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->decompress_cnt);
atomic64_add(req->slen, &tfm->base.__crt_alg->decompress_tlen);
}
#endif
diff --git a/include/crypto/aead.h b/include/crypto/aead.h
index 0d765d7bfb82..99afd78c665d 100644
--- a/include/crypto/aead.h
+++ b/include/crypto/aead.h
@@ -312,9 +312,9 @@ static inline void crypto_stat_aead_encrypt(struct aead_request *req, int ret)
struct crypto_aead *tfm = crypto_aead_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->aead_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->aead_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->encrypt_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->encrypt_cnt);
atomic64_add(req->cryptlen, &tfm->base.__crt_alg->encrypt_tlen);
}
#endif
@@ -326,9 +326,9 @@ static inline void crypto_stat_aead_decrypt(struct aead_request *req, int ret)
struct crypto_aead *tfm = crypto_aead_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->aead_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->aead_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->decrypt_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->decrypt_cnt);
atomic64_add(req->cryptlen, &tfm->base.__crt_alg->decrypt_tlen);
}
#endif
diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index ee072a82d39d..97056fd5e718 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -278,9 +278,9 @@ static inline void crypto_stat_akcipher_encrypt(struct akcipher_request *req,
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->encrypt_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->encrypt_cnt);
atomic64_add(req->src_len, &tfm->base.__crt_alg->encrypt_tlen);
}
#endif
@@ -293,9 +293,9 @@ static inline void crypto_stat_akcipher_decrypt(struct akcipher_request *req,
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->decrypt_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->decrypt_cnt);
atomic64_add(req->src_len, &tfm->base.__crt_alg->decrypt_tlen);
}
#endif
@@ -308,9 +308,9 @@ static inline void crypto_stat_akcipher_sign(struct akcipher_request *req,
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
else
- atomic_inc(&tfm->base.__crt_alg->sign_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->sign_cnt);
#endif
}
@@ -321,9 +321,9 @@ static inline void crypto_stat_akcipher_verify(struct akcipher_request *req,
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
else
- atomic_inc(&tfm->base.__crt_alg->verify_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->verify_cnt);
#endif
}
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index bc7796600338..52920bed05ba 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -418,7 +418,7 @@ static inline void crypto_stat_ahash_update(struct ahash_request *req, int ret)
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic_inc(&tfm->base.__crt_alg->hash_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->hash_err_cnt);
else
atomic64_add(req->nbytes, &tfm->base.__crt_alg->hash_tlen);
#endif
@@ -430,9 +430,9 @@ static inline void crypto_stat_ahash_final(struct ahash_request *req, int ret)
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->hash_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->hash_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->hash_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->hash_cnt);
atomic64_add(req->nbytes, &tfm->base.__crt_alg->hash_tlen);
}
#endif
diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index f517ba6d3a27..bd5103a80919 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -272,9 +272,9 @@ static inline void crypto_stat_kpp_set_secret(struct crypto_kpp *tfm, int ret)
{
#ifdef CONFIG_CRYPTO_STATS
if (ret)
- atomic_inc(&tfm->base.__crt_alg->kpp_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
else
- atomic_inc(&tfm->base.__crt_alg->setsecret_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->setsecret_cnt);
#endif
}
@@ -285,9 +285,9 @@ static inline void crypto_stat_kpp_generate_public_key(struct kpp_request *req,
struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
if (ret)
- atomic_inc(&tfm->base.__crt_alg->kpp_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
else
- atomic_inc(&tfm->base.__crt_alg->generate_public_key_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->generate_public_key_cnt);
#endif
}
@@ -298,9 +298,9 @@ static inline void crypto_stat_kpp_compute_shared_secret(struct kpp_request *req
struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
if (ret)
- atomic_inc(&tfm->base.__crt_alg->kpp_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
else
- atomic_inc(&tfm->base.__crt_alg->compute_shared_secret_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->compute_shared_secret_cnt);
#endif
}
diff --git a/include/crypto/rng.h b/include/crypto/rng.h
index 0d781fa77161..bdcaac34e9d7 100644
--- a/include/crypto/rng.h
+++ b/include/crypto/rng.h
@@ -126,9 +126,9 @@ static inline void crypto_stat_rng_seed(struct crypto_rng *tfm, int ret)
{
#ifdef CONFIG_CRYPTO_STATS
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic_inc(&tfm->base.__crt_alg->rng_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->rng_err_cnt);
else
- atomic_inc(&tfm->base.__crt_alg->seed_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->seed_cnt);
#endif
}
@@ -137,9 +137,9 @@ static inline void crypto_stat_rng_generate(struct crypto_rng *tfm,
{
#ifdef CONFIG_CRYPTO_STATS
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&tfm->base.__crt_alg->rng_err_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->rng_err_cnt);
} else {
- atomic_inc(&tfm->base.__crt_alg->generate_cnt);
+ atomic64_inc(&tfm->base.__crt_alg->generate_cnt);
atomic64_add(dlen, &tfm->base.__crt_alg->generate_tlen);
}
#endif
diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
index 925f547cdcfa..dff54731ddf4 100644
--- a/include/crypto/skcipher.h
+++ b/include/crypto/skcipher.h
@@ -491,9 +491,9 @@ static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
{
#ifdef CONFIG_CRYPTO_STATS
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&alg->cipher_err_cnt);
+ atomic64_inc(&alg->cipher_err_cnt);
} else {
- atomic_inc(&alg->encrypt_cnt);
+ atomic64_inc(&alg->encrypt_cnt);
atomic64_add(req->cryptlen, &alg->encrypt_tlen);
}
#endif
@@ -504,9 +504,9 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
{
#ifdef CONFIG_CRYPTO_STATS
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&alg->cipher_err_cnt);
+ atomic64_inc(&alg->cipher_err_cnt);
} else {
- atomic_inc(&alg->decrypt_cnt);
+ atomic64_inc(&alg->decrypt_cnt);
atomic64_add(req->cryptlen, &alg->decrypt_tlen);
}
#endif
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 35de61d99cd5..7d913330402e 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -517,11 +517,11 @@ struct crypto_alg {
#ifdef CONFIG_CRYPTO_STATS
union {
- atomic_t encrypt_cnt;
- atomic_t compress_cnt;
- atomic_t generate_cnt;
- atomic_t hash_cnt;
- atomic_t setsecret_cnt;
+ atomic64_t encrypt_cnt;
+ atomic64_t compress_cnt;
+ atomic64_t generate_cnt;
+ atomic64_t hash_cnt;
+ atomic64_t setsecret_cnt;
};
union {
atomic64_t encrypt_tlen;
@@ -530,29 +530,29 @@ struct crypto_alg {
atomic64_t hash_tlen;
};
union {
- atomic_t akcipher_err_cnt;
- atomic_t cipher_err_cnt;
- atomic_t compress_err_cnt;
- atomic_t aead_err_cnt;
- atomic_t hash_err_cnt;
- atomic_t rng_err_cnt;
- atomic_t kpp_err_cnt;
+ atomic64_t akcipher_err_cnt;
+ atomic64_t cipher_err_cnt;
+ atomic64_t compress_err_cnt;
+ atomic64_t aead_err_cnt;
+ atomic64_t hash_err_cnt;
+ atomic64_t rng_err_cnt;
+ atomic64_t kpp_err_cnt;
};
union {
- atomic_t decrypt_cnt;
- atomic_t decompress_cnt;
- atomic_t seed_cnt;
- atomic_t generate_public_key_cnt;
+ atomic64_t decrypt_cnt;
+ atomic64_t decompress_cnt;
+ atomic64_t seed_cnt;
+ atomic64_t generate_public_key_cnt;
};
union {
atomic64_t decrypt_tlen;
atomic64_t decompress_tlen;
};
union {
- atomic_t verify_cnt;
- atomic_t compute_shared_secret_cnt;
+ atomic64_t verify_cnt;
+ atomic64_t compute_shared_secret_cnt;
};
- atomic_t sign_cnt;
+ atomic64_t sign_cnt;
#endif
} CRYPTO_MINALIGN_ATTR;
@@ -983,9 +983,9 @@ static inline void crypto_stat_ablkcipher_encrypt(struct ablkcipher_request *req
crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
+ atomic64_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
} else {
- atomic_inc(&crt->base->base.__crt_alg->encrypt_cnt);
+ atomic64_inc(&crt->base->base.__crt_alg->encrypt_cnt);
atomic64_add(req->nbytes, &crt->base->base.__crt_alg->encrypt_tlen);
}
#endif
@@ -999,9 +999,9 @@ static inline void crypto_stat_ablkcipher_decrypt(struct ablkcipher_request *req
crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
+ atomic64_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
} else {
- atomic_inc(&crt->base->base.__crt_alg->decrypt_cnt);
+ atomic64_inc(&crt->base->base.__crt_alg->decrypt_cnt);
atomic64_add(req->nbytes, &crt->base->base.__crt_alg->decrypt_tlen);
}
#endif
diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
index 6dafbc3e4414..9f8187077ce4 100644
--- a/include/uapi/linux/cryptouser.h
+++ b/include/uapi/linux/cryptouser.h
@@ -79,11 +79,11 @@ struct crypto_user_alg {
struct crypto_stat {
char type[CRYPTO_MAX_NAME];
union {
- __u32 stat_encrypt_cnt;
- __u32 stat_compress_cnt;
- __u32 stat_generate_cnt;
- __u32 stat_hash_cnt;
- __u32 stat_setsecret_cnt;
+ __u64 stat_encrypt_cnt;
+ __u64 stat_compress_cnt;
+ __u64 stat_generate_cnt;
+ __u64 stat_hash_cnt;
+ __u64 stat_setsecret_cnt;
};
union {
__u64 stat_encrypt_tlen;
@@ -92,29 +92,29 @@ struct crypto_stat {
__u64 stat_hash_tlen;
};
union {
- __u32 stat_akcipher_err_cnt;
- __u32 stat_cipher_err_cnt;
- __u32 stat_compress_err_cnt;
- __u32 stat_aead_err_cnt;
- __u32 stat_hash_err_cnt;
- __u32 stat_rng_err_cnt;
- __u32 stat_kpp_err_cnt;
+ __u64 stat_akcipher_err_cnt;
+ __u64 stat_cipher_err_cnt;
+ __u64 stat_compress_err_cnt;
+ __u64 stat_aead_err_cnt;
+ __u64 stat_hash_err_cnt;
+ __u64 stat_rng_err_cnt;
+ __u64 stat_kpp_err_cnt;
};
union {
- __u32 stat_decrypt_cnt;
- __u32 stat_decompress_cnt;
- __u32 stat_seed_cnt;
- __u32 stat_generate_public_key_cnt;
+ __u64 stat_decrypt_cnt;
+ __u64 stat_decompress_cnt;
+ __u64 stat_seed_cnt;
+ __u64 stat_generate_public_key_cnt;
};
union {
__u64 stat_decrypt_tlen;
__u64 stat_decompress_tlen;
};
union {
- __u32 stat_verify_cnt;
- __u32 stat_compute_shared_secret_cnt;
+ __u64 stat_verify_cnt;
+ __u64 stat_compute_shared_secret_cnt;
};
- __u32 stat_sign_cnt;
+ __u64 stat_sign_cnt;
};
struct crypto_report_larval {
--
2.18.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures
2018-11-05 12:51 [PATCH 0/5] crypto: crypto_user_stat: misc enhancement Corentin Labbe
2018-11-05 12:51 ` [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional Corentin Labbe
2018-11-05 12:51 ` [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64 Corentin Labbe
@ 2018-11-05 12:51 ` Corentin Labbe
2018-11-06 1:44 ` Eric Biggers
2018-11-05 12:51 ` [PATCH 4/5] crypto: tool: getstat: convert user space example to the new crypto_user_stat uapi Corentin Labbe
2018-11-05 12:51 ` [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request Corentin Labbe
4 siblings, 1 reply; 14+ messages in thread
From: Corentin Labbe @ 2018-11-05 12:51 UTC (permalink / raw)
To: davem, ebiggers, herbert; +Cc: linux-crypto, linux-kernel, Corentin Labbe
It is cleaner to have each stat in their own structures.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
crypto/crypto_user_stat.c | 20 +++----
include/uapi/linux/cryptouser.h | 101 ++++++++++++++++++++------------
2 files changed, 73 insertions(+), 48 deletions(-)
diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
index 352569f378a0..3c14be2f7a1b 100644
--- a/crypto/crypto_user_stat.c
+++ b/crypto/crypto_user_stat.c
@@ -33,7 +33,7 @@ struct crypto_dump_info {
static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat raead;
+ struct crypto_stat_aead raead;
u64 v64;
memset(&raead, 0, sizeof(raead));
@@ -56,7 +56,7 @@ static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rcipher;
+ struct crypto_stat_cipher rcipher;
u64 v64;
memset(&rcipher, 0, sizeof(rcipher));
@@ -79,7 +79,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rcomp;
+ struct crypto_stat_compress rcomp;
u64 v64;
memset(&rcomp, 0, sizeof(rcomp));
@@ -101,7 +101,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat racomp;
+ struct crypto_stat_compress racomp;
u64 v64;
memset(&racomp, 0, sizeof(racomp));
@@ -123,7 +123,7 @@ static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rakcipher;
+ struct crypto_stat_akcipher rakcipher;
u64 v64;
memset(&rakcipher, 0, sizeof(rakcipher));
@@ -150,7 +150,7 @@ static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rkpp;
+ struct crypto_stat_kpp rkpp;
u64 v;
memset(&rkpp, 0, sizeof(rkpp));
@@ -171,7 +171,7 @@ static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rhash;
+ struct crypto_stat_hash rhash;
u64 v64;
memset(&rhash, 0, sizeof(rhash));
@@ -190,7 +190,7 @@ static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rhash;
+ struct crypto_stat_hash rhash;
u64 v64;
memset(&rhash, 0, sizeof(rhash));
@@ -209,7 +209,7 @@ static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
static int crypto_report_rng(struct sk_buff *skb, struct crypto_alg *alg)
{
- struct crypto_stat rrng;
+ struct crypto_stat_rng rrng;
u64 v64;
memset(&rrng, 0, sizeof(rrng));
@@ -248,7 +248,7 @@ static int crypto_reportstat_one(struct crypto_alg *alg,
if (nla_put_u32(skb, CRYPTOCFGA_PRIORITY_VAL, alg->cra_priority))
goto nla_put_failure;
if (alg->cra_flags & CRYPTO_ALG_LARVAL) {
- struct crypto_stat rl;
+ struct crypto_stat_larval rl;
memset(&rl, 0, sizeof(rl));
strscpy(rl.type, "larval", sizeof(rl.type));
diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
index 9f8187077ce4..790b5c6511e5 100644
--- a/include/uapi/linux/cryptouser.h
+++ b/include/uapi/linux/cryptouser.h
@@ -76,47 +76,72 @@ struct crypto_user_alg {
__u32 cru_flags;
};
-struct crypto_stat {
- char type[CRYPTO_MAX_NAME];
- union {
- __u64 stat_encrypt_cnt;
- __u64 stat_compress_cnt;
- __u64 stat_generate_cnt;
- __u64 stat_hash_cnt;
- __u64 stat_setsecret_cnt;
- };
- union {
- __u64 stat_encrypt_tlen;
- __u64 stat_compress_tlen;
- __u64 stat_generate_tlen;
- __u64 stat_hash_tlen;
- };
- union {
- __u64 stat_akcipher_err_cnt;
- __u64 stat_cipher_err_cnt;
- __u64 stat_compress_err_cnt;
- __u64 stat_aead_err_cnt;
- __u64 stat_hash_err_cnt;
- __u64 stat_rng_err_cnt;
- __u64 stat_kpp_err_cnt;
- };
- union {
- __u64 stat_decrypt_cnt;
- __u64 stat_decompress_cnt;
- __u64 stat_seed_cnt;
- __u64 stat_generate_public_key_cnt;
- };
- union {
- __u64 stat_decrypt_tlen;
- __u64 stat_decompress_tlen;
- };
- union {
- __u64 stat_verify_cnt;
- __u64 stat_compute_shared_secret_cnt;
- };
+struct crypto_stat_cipher {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_encrypt_cnt;
+ __u64 stat_encrypt_tlen;
+ __u64 stat_decrypt_cnt;
+ __u64 stat_decrypt_tlen;
+ __u64 stat_cipher_err_cnt;
+};
+
+struct crypto_stat_aead {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_encrypt_cnt;
+ __u64 stat_encrypt_tlen;
+ __u64 stat_decrypt_cnt;
+ __u64 stat_decrypt_tlen;
+ __u64 stat_cipher_err_cnt;
+ __u64 stat_aead_err_cnt;
+};
+
+struct crypto_stat_akcipher {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_encrypt_cnt;
+ __u64 stat_encrypt_tlen;
+ __u64 stat_decrypt_cnt;
+ __u64 stat_decrypt_tlen;
+ __u64 stat_akcipher_err_cnt;
+ __u64 stat_verify_cnt;
__u64 stat_sign_cnt;
};
+struct crypto_stat_compress {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_compress_cnt;
+ __u64 stat_compress_tlen;
+ __u64 stat_decompress_cnt;
+ __u64 stat_decompress_tlen;
+ __u64 stat_compress_err_cnt;
+};
+
+struct crypto_stat_rng {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_generate_cnt;
+ __u64 stat_generate_tlen;
+ __u64 stat_rng_err_cnt;
+ __u64 stat_seed_cnt;
+};
+
+struct crypto_stat_hash {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_hash_cnt;
+ __u64 stat_hash_tlen;
+ __u64 stat_hash_err_cnt;
+};
+
+struct crypto_stat_kpp {
+ char type[CRYPTO_MAX_NAME];
+ __u64 stat_setsecret_cnt;
+ __u64 stat_kpp_err_cnt;
+ __u64 stat_generate_public_key_cnt;
+ __u64 stat_compute_shared_secret_cnt;
+};
+
+struct crypto_stat_larval {
+ char type[CRYPTO_MAX_NAME];
+};
+
struct crypto_report_larval {
char type[CRYPTO_MAX_NAME];
};
--
2.18.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] crypto: tool: getstat: convert user space example to the new crypto_user_stat uapi
2018-11-05 12:51 [PATCH 0/5] crypto: crypto_user_stat: misc enhancement Corentin Labbe
` (2 preceding siblings ...)
2018-11-05 12:51 ` [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures Corentin Labbe
@ 2018-11-05 12:51 ` Corentin Labbe
2018-11-05 12:51 ` [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request Corentin Labbe
4 siblings, 0 replies; 14+ messages in thread
From: Corentin Labbe @ 2018-11-05 12:51 UTC (permalink / raw)
To: davem, ebiggers, herbert; +Cc: linux-crypto, linux-kernel, Corentin Labbe
This patch converts the getstat example tool to the recent changes done in crypto_user_stat
- changed all stats to u64
- separated struct stats for each crypto alg
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
tools/crypto/getstat.c | 54 +++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/tools/crypto/getstat.c b/tools/crypto/getstat.c
index 24115173a483..57fbb94608d4 100644
--- a/tools/crypto/getstat.c
+++ b/tools/crypto/getstat.c
@@ -152,53 +152,53 @@ static int get_stat(const char *drivername)
if (tb[CRYPTOCFGA_STAT_HASH]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_HASH];
- struct crypto_stat *rhash =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tHash\n\tHash: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_hash *rhash =
+ (struct crypto_stat_hash *)RTA_DATA(rta);
+ printf("%s\tHash\n\tHash: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
rhash->stat_hash_cnt, rhash->stat_hash_tlen,
rhash->stat_hash_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_COMPRESS]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_COMPRESS];
- struct crypto_stat *rblk =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tCompress\n\tCompress: %u bytes: %llu\n\tDecompress: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_compress *rblk =
+ (struct crypto_stat_compress *)RTA_DATA(rta);
+ printf("%s\tCompress\n\tCompress: %llu bytes: %llu\n\tDecompress: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
rblk->stat_compress_cnt, rblk->stat_compress_tlen,
rblk->stat_decompress_cnt, rblk->stat_decompress_tlen,
rblk->stat_compress_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_ACOMP]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_ACOMP];
- struct crypto_stat *rcomp =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tACompress\n\tCompress: %u bytes: %llu\n\tDecompress: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_compress *rcomp =
+ (struct crypto_stat_compress *)RTA_DATA(rta);
+ printf("%s\tACompress\n\tCompress: %llu bytes: %llu\n\tDecompress: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
rcomp->stat_compress_cnt, rcomp->stat_compress_tlen,
rcomp->stat_decompress_cnt, rcomp->stat_decompress_tlen,
rcomp->stat_compress_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_AEAD]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_AEAD];
- struct crypto_stat *raead =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tAEAD\n\tEncrypt: %u bytes: %llu\n\tDecrypt: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_aead *raead =
+ (struct crypto_stat_aead *)RTA_DATA(rta);
+ printf("%s\tAEAD\n\tEncrypt: %llu bytes: %llu\n\tDecrypt: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
raead->stat_encrypt_cnt, raead->stat_encrypt_tlen,
raead->stat_decrypt_cnt, raead->stat_decrypt_tlen,
raead->stat_aead_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_BLKCIPHER]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_BLKCIPHER];
- struct crypto_stat *rblk =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tCipher\n\tEncrypt: %u bytes: %llu\n\tDecrypt: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_cipher *rblk =
+ (struct crypto_stat_cipher *)RTA_DATA(rta);
+ printf("%s\tCipher\n\tEncrypt: %llu bytes: %llu\n\tDecrypt: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
rblk->stat_encrypt_cnt, rblk->stat_encrypt_tlen,
rblk->stat_decrypt_cnt, rblk->stat_decrypt_tlen,
rblk->stat_cipher_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_AKCIPHER]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_AKCIPHER];
- struct crypto_stat *rblk =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tAkcipher\n\tEncrypt: %u bytes: %llu\n\tDecrypt: %u bytes: %llu\n\tSign: %u\n\tVerify: %u\n\tErrors: %u\n",
+ struct crypto_stat_akcipher *rblk =
+ (struct crypto_stat_akcipher *)RTA_DATA(rta);
+ printf("%s\tAkcipher\n\tEncrypt: %llu bytes: %llu\n\tDecrypt: %llu bytes: %llu\n\tSign: %llu\n\tVerify: %llu\n\tErrors: %llu\n",
drivername,
rblk->stat_encrypt_cnt, rblk->stat_encrypt_tlen,
rblk->stat_decrypt_cnt, rblk->stat_decrypt_tlen,
@@ -206,27 +206,27 @@ static int get_stat(const char *drivername)
rblk->stat_akcipher_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_CIPHER]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_CIPHER];
- struct crypto_stat *rblk =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tcipher\n\tEncrypt: %u bytes: %llu\n\tDecrypt: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_cipher *rblk =
+ (struct crypto_stat_cipher *)RTA_DATA(rta);
+ printf("%s\tcipher\n\tEncrypt: %llu bytes: %llu\n\tDecrypt: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
rblk->stat_encrypt_cnt, rblk->stat_encrypt_tlen,
rblk->stat_decrypt_cnt, rblk->stat_decrypt_tlen,
rblk->stat_cipher_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_RNG]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_RNG];
- struct crypto_stat *rrng =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tRNG\n\tSeed: %u\n\tGenerate: %u bytes: %llu\n\tErrors: %u\n",
+ struct crypto_stat_rng *rrng =
+ (struct crypto_stat_rng *)RTA_DATA(rta);
+ printf("%s\tRNG\n\tSeed: %llu\n\tGenerate: %llu bytes: %llu\n\tErrors: %llu\n",
drivername,
rrng->stat_seed_cnt,
rrng->stat_generate_cnt, rrng->stat_generate_tlen,
rrng->stat_rng_err_cnt);
} else if (tb[CRYPTOCFGA_STAT_KPP]) {
struct rtattr *rta = tb[CRYPTOCFGA_STAT_KPP];
- struct crypto_stat *rkpp =
- (struct crypto_stat *)RTA_DATA(rta);
- printf("%s\tKPP\n\tSetsecret: %u\n\tGenerate public key: %u\n\tCompute_shared_secret: %u\n\tErrors: %u\n",
+ struct crypto_stat_kpp *rkpp =
+ (struct crypto_stat_kpp *)RTA_DATA(rta);
+ printf("%s\tKPP\n\tSetsecret: %llu\n\tGenerate public key: %llu\n\tCompute_shared_secret: %llu\n\tErrors: %llu\n",
drivername,
rkpp->stat_setsecret_cnt,
rkpp->stat_generate_public_key_cnt,
--
2.18.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request
2018-11-05 12:51 [PATCH 0/5] crypto: crypto_user_stat: misc enhancement Corentin Labbe
` (3 preceding siblings ...)
2018-11-05 12:51 ` [PATCH 4/5] crypto: tool: getstat: convert user space example to the new crypto_user_stat uapi Corentin Labbe
@ 2018-11-05 12:51 ` Corentin Labbe
2018-11-06 1:49 ` Eric Biggers
4 siblings, 1 reply; 14+ messages in thread
From: Corentin Labbe @ 2018-11-05 12:51 UTC (permalink / raw)
To: davem, ebiggers, herbert; +Cc: linux-crypto, linux-kernel, Corentin Labbe
All crypto_stats functions use the struct xxx_request for feeding stats,
but in some case this structure could already be freed.
For fixing this, the needed parameters (len and alg) will be stored
before the request being executed.
Fixes: cac5818c25d0 ("crypto: user - Implement a generic crypto statistics")
Reported-by: syzbot <syzbot+6939a606a5305e9e9799@syzkaller.appspotmail.com>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
crypto/ahash.c | 14 ++++++++--
include/crypto/acompress.h | 30 ++++++++++----------
include/crypto/aead.h | 30 ++++++++++----------
include/crypto/akcipher.h | 56 ++++++++++++++++++--------------------
include/crypto/hash.h | 25 +++++++++--------
include/crypto/kpp.h | 22 +++++++--------
include/crypto/skcipher.h | 16 +++++++----
include/linux/crypto.h | 34 +++++++++++------------
8 files changed, 118 insertions(+), 109 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a348fbcf8f9..3f4c44c1e80f 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -364,20 +364,26 @@ static int crypto_ahash_op(struct ahash_request *req,
int crypto_ahash_final(struct ahash_request *req)
{
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int nbytes = req->nbytes;
int ret;
ret = crypto_ahash_op(req, crypto_ahash_reqtfm(req)->final);
- crypto_stat_ahash_final(req, ret);
+ crypto_stat_ahash_final(nbytes, ret, alg);
return ret;
}
EXPORT_SYMBOL_GPL(crypto_ahash_final);
int crypto_ahash_finup(struct ahash_request *req)
{
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int nbytes = req->nbytes;
int ret;
ret = crypto_ahash_op(req, crypto_ahash_reqtfm(req)->finup);
- crypto_stat_ahash_final(req, ret);
+ crypto_stat_ahash_final(nbytes, ret, alg);
return ret;
}
EXPORT_SYMBOL_GPL(crypto_ahash_finup);
@@ -385,13 +391,15 @@ EXPORT_SYMBOL_GPL(crypto_ahash_finup);
int crypto_ahash_digest(struct ahash_request *req)
{
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int nbytes = req->nbytes;
int ret;
if (crypto_ahash_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = crypto_ahash_op(req, tfm->digest);
- crypto_stat_ahash_final(req, ret);
+ crypto_stat_ahash_final(nbytes, ret, alg);
return ret;
}
EXPORT_SYMBOL_GPL(crypto_ahash_digest);
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index f79918196811..396407cc34c7 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -234,30 +234,28 @@ static inline void acomp_request_set_params(struct acomp_req *req,
req->flags |= CRYPTO_ACOMP_ALLOC_OUTPUT;
}
-static inline void crypto_stat_compress(struct acomp_req *req, int ret)
+static inline void crypto_stat_compress(unsigned int slen, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->compress_err_cnt);
+ atomic64_inc(&alg->compress_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->compress_cnt);
- atomic64_add(req->slen, &tfm->base.__crt_alg->compress_tlen);
+ atomic64_inc(&alg->compress_cnt);
+ atomic64_add(slen, &alg->compress_tlen);
}
#endif
}
-static inline void crypto_stat_decompress(struct acomp_req *req, int ret)
+static inline void crypto_stat_decompress(unsigned int slen, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->compress_err_cnt);
+ atomic64_inc(&alg->compress_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->decompress_cnt);
- atomic64_add(req->slen, &tfm->base.__crt_alg->decompress_tlen);
+ atomic64_inc(&alg->decompress_cnt);
+ atomic64_add(slen, &alg->decompress_tlen);
}
#endif
}
@@ -274,10 +272,12 @@ static inline void crypto_stat_decompress(struct acomp_req *req, int ret)
static inline int crypto_acomp_compress(struct acomp_req *req)
{
struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int slen = req->slen;
int ret;
ret = tfm->compress(req);
- crypto_stat_compress(req, ret);
+ crypto_stat_compress(slen, ret, alg);
return ret;
}
@@ -293,10 +293,12 @@ static inline int crypto_acomp_compress(struct acomp_req *req)
static inline int crypto_acomp_decompress(struct acomp_req *req)
{
struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int slen = req->slen;
int ret;
ret = tfm->decompress(req);
- crypto_stat_decompress(req, ret);
+ crypto_stat_decompress(slen, ret, alg);
return ret;
}
diff --git a/include/crypto/aead.h b/include/crypto/aead.h
index 99afd78c665d..c959cd39049d 100644
--- a/include/crypto/aead.h
+++ b/include/crypto/aead.h
@@ -306,30 +306,28 @@ static inline struct crypto_aead *crypto_aead_reqtfm(struct aead_request *req)
return __crypto_aead_cast(req->base.tfm);
}
-static inline void crypto_stat_aead_encrypt(struct aead_request *req, int ret)
+static inline void crypto_stat_aead_encrypt(unsigned int cryptlen,
+ struct crypto_alg *alg, int ret)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_aead *tfm = crypto_aead_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->aead_err_cnt);
+ atomic64_inc(&alg->aead_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->encrypt_cnt);
- atomic64_add(req->cryptlen, &tfm->base.__crt_alg->encrypt_tlen);
+ atomic64_inc(&alg->encrypt_cnt);
+ atomic64_add(cryptlen, &alg->encrypt_tlen);
}
#endif
}
-static inline void crypto_stat_aead_decrypt(struct aead_request *req, int ret)
+static inline void crypto_stat_aead_decrypt(unsigned int cryptlen,
+ struct crypto_alg *alg, int ret)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_aead *tfm = crypto_aead_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->aead_err_cnt);
+ atomic64_inc(&alg->aead_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->decrypt_cnt);
- atomic64_add(req->cryptlen, &tfm->base.__crt_alg->decrypt_tlen);
+ atomic64_inc(&alg->decrypt_cnt);
+ atomic64_add(cryptlen, &alg->decrypt_tlen);
}
#endif
}
@@ -356,13 +354,15 @@ static inline void crypto_stat_aead_decrypt(struct aead_request *req, int ret)
static inline int crypto_aead_encrypt(struct aead_request *req)
{
struct crypto_aead *aead = crypto_aead_reqtfm(req);
+ struct crypto_alg *alg = aead->base.__crt_alg;
+ unsigned int cryptlen = req->cryptlen;
int ret;
if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = crypto_aead_alg(aead)->encrypt(req);
- crypto_stat_aead_encrypt(req, ret);
+ crypto_stat_aead_encrypt(cryptlen, alg, ret);
return ret;
}
@@ -391,6 +391,8 @@ static inline int crypto_aead_encrypt(struct aead_request *req)
static inline int crypto_aead_decrypt(struct aead_request *req)
{
struct crypto_aead *aead = crypto_aead_reqtfm(req);
+ struct crypto_alg *alg = aead->base.__crt_alg;
+ unsigned int cryptlen = req->cryptlen;
int ret;
if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
@@ -399,7 +401,7 @@ static inline int crypto_aead_decrypt(struct aead_request *req)
ret = -EINVAL;
else
ret = crypto_aead_alg(aead)->decrypt(req);
- crypto_stat_aead_decrypt(req, ret);
+ crypto_stat_aead_decrypt(cryptlen, alg, ret);
return ret;
}
diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index 97056fd5e718..f20cdb775674 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -271,59 +271,49 @@ static inline unsigned int crypto_akcipher_maxsize(struct crypto_akcipher *tfm)
return alg->max_size(tfm);
}
-static inline void crypto_stat_akcipher_encrypt(struct akcipher_request *req,
- int ret)
+static inline void crypto_stat_akcipher_encrypt(unsigned int src_len, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&alg->akcipher_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->encrypt_cnt);
- atomic64_add(req->src_len, &tfm->base.__crt_alg->encrypt_tlen);
+ atomic64_inc(&alg->encrypt_cnt);
+ atomic64_add(src_len, &alg->encrypt_tlen);
}
#endif
}
-static inline void crypto_stat_akcipher_decrypt(struct akcipher_request *req,
- int ret)
+static inline void crypto_stat_akcipher_decrypt(unsigned int src_len, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&alg->akcipher_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->decrypt_cnt);
- atomic64_add(req->src_len, &tfm->base.__crt_alg->decrypt_tlen);
+ atomic64_inc(&alg->decrypt_cnt);
+ atomic64_add(src_len, &alg->decrypt_tlen);
}
#endif
}
-static inline void crypto_stat_akcipher_sign(struct akcipher_request *req,
- int ret)
+static inline void crypto_stat_akcipher_sign(int ret, struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&alg->akcipher_err_cnt);
else
- atomic64_inc(&tfm->base.__crt_alg->sign_cnt);
+ atomic64_inc(&alg->sign_cnt);
#endif
}
-static inline void crypto_stat_akcipher_verify(struct akcipher_request *req,
- int ret)
+static inline void crypto_stat_akcipher_verify(int ret, struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
+ atomic64_inc(&alg->akcipher_err_cnt);
else
- atomic64_inc(&tfm->base.__crt_alg->verify_cnt);
+ atomic64_inc(&alg->verify_cnt);
#endif
}
@@ -341,10 +331,12 @@ static inline int crypto_akcipher_encrypt(struct akcipher_request *req)
{
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
+ struct crypto_alg *calg = tfm->base.__crt_alg;
+ unsigned int src_len = req->src_len;
int ret;
ret = alg->encrypt(req);
- crypto_stat_akcipher_encrypt(req, ret);
+ crypto_stat_akcipher_encrypt(src_len, ret, calg);
return ret;
}
@@ -362,10 +354,12 @@ static inline int crypto_akcipher_decrypt(struct akcipher_request *req)
{
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
+ struct crypto_alg *calg = tfm->base.__crt_alg;
+ unsigned int src_len = req->src_len;
int ret;
ret = alg->decrypt(req);
- crypto_stat_akcipher_decrypt(req, ret);
+ crypto_stat_akcipher_decrypt(src_len, ret, calg);
return ret;
}
@@ -383,10 +377,11 @@ static inline int crypto_akcipher_sign(struct akcipher_request *req)
{
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
+ struct crypto_alg *calg = tfm->base.__crt_alg;
int ret;
ret = alg->sign(req);
- crypto_stat_akcipher_sign(req, ret);
+ crypto_stat_akcipher_sign(ret, calg);
return ret;
}
@@ -404,10 +399,11 @@ static inline int crypto_akcipher_verify(struct akcipher_request *req)
{
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
+ struct crypto_alg *calg = tfm->base.__crt_alg;
int ret;
ret = alg->verify(req);
- crypto_stat_akcipher_verify(req, ret);
+ crypto_stat_akcipher_verify(ret, calg);
return ret;
}
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 52920bed05ba..83fe47f34db1 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -412,28 +412,26 @@ static inline void *ahash_request_ctx(struct ahash_request *req)
int crypto_ahash_setkey(struct crypto_ahash *tfm, const u8 *key,
unsigned int keylen);
-static inline void crypto_stat_ahash_update(struct ahash_request *req, int ret)
+static inline void crypto_stat_ahash_update(unsigned int nbytes, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY)
- atomic64_inc(&tfm->base.__crt_alg->hash_err_cnt);
+ atomic64_inc(&alg->hash_err_cnt);
else
- atomic64_add(req->nbytes, &tfm->base.__crt_alg->hash_tlen);
+ atomic64_add(nbytes, &alg->hash_tlen);
#endif
}
-static inline void crypto_stat_ahash_final(struct ahash_request *req, int ret)
+static inline void crypto_stat_ahash_final(unsigned int nbytes, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&tfm->base.__crt_alg->hash_err_cnt);
+ atomic64_inc(&alg->hash_err_cnt);
} else {
- atomic64_inc(&tfm->base.__crt_alg->hash_cnt);
- atomic64_add(req->nbytes, &tfm->base.__crt_alg->hash_tlen);
+ atomic64_inc(&alg->hash_cnt);
+ atomic64_add(nbytes, &alg->hash_tlen);
}
#endif
}
@@ -552,10 +550,13 @@ static inline int crypto_ahash_init(struct ahash_request *req)
*/
static inline int crypto_ahash_update(struct ahash_request *req)
{
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int nbytes = req->nbytes;
int ret;
ret = crypto_ahash_reqtfm(req)->update(req);
- crypto_stat_ahash_update(req, ret);
+ crypto_stat_ahash_update(nbytes, ret, alg);
return ret;
}
diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index bd5103a80919..525689505b49 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -278,29 +278,25 @@ static inline void crypto_stat_kpp_set_secret(struct crypto_kpp *tfm, int ret)
#endif
}
-static inline void crypto_stat_kpp_generate_public_key(struct kpp_request *req,
+static inline void crypto_stat_kpp_generate_public_key(struct crypto_alg *alg,
int ret)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
-
if (ret)
- atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
+ atomic64_inc(&alg->kpp_err_cnt);
else
- atomic64_inc(&tfm->base.__crt_alg->generate_public_key_cnt);
+ atomic64_inc(&alg->generate_public_key_cnt);
#endif
}
-static inline void crypto_stat_kpp_compute_shared_secret(struct kpp_request *req,
+static inline void crypto_stat_kpp_compute_shared_secret(struct crypto_alg *alg,
int ret)
{
#ifdef CONFIG_CRYPTO_STATS
- struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
-
if (ret)
- atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
+ atomic64_inc(&alg->kpp_err_cnt);
else
- atomic64_inc(&tfm->base.__crt_alg->compute_shared_secret_cnt);
+ atomic64_inc(&alg->compute_shared_secret_cnt);
#endif
}
@@ -347,10 +343,11 @@ static inline int crypto_kpp_generate_public_key(struct kpp_request *req)
{
struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
struct kpp_alg *alg = crypto_kpp_alg(tfm);
+ struct crypto_alg *calg = tfm->base.__crt_alg;
int ret;
ret = alg->generate_public_key(req);
- crypto_stat_kpp_generate_public_key(req, ret);
+ crypto_stat_kpp_generate_public_key(calg, ret);
return ret;
}
@@ -368,10 +365,11 @@ static inline int crypto_kpp_compute_shared_secret(struct kpp_request *req)
{
struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
struct kpp_alg *alg = crypto_kpp_alg(tfm);
+ struct crypto_alg *calg = tfm->base.__crt_alg;
int ret;
ret = alg->compute_shared_secret(req);
- crypto_stat_kpp_compute_shared_secret(req, ret);
+ crypto_stat_kpp_compute_shared_secret(calg, ret);
return ret;
}
diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
index dff54731ddf4..a68b227de96c 100644
--- a/include/crypto/skcipher.h
+++ b/include/crypto/skcipher.h
@@ -486,7 +486,7 @@ static inline struct crypto_sync_skcipher *crypto_sync_skcipher_reqtfm(
return container_of(tfm, struct crypto_sync_skcipher, base);
}
-static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
+static inline void crypto_stat_skcipher_encrypt(unsigned int cryptlen,
int ret, struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
@@ -494,12 +494,12 @@ static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
atomic64_inc(&alg->cipher_err_cnt);
} else {
atomic64_inc(&alg->encrypt_cnt);
- atomic64_add(req->cryptlen, &alg->encrypt_tlen);
+ atomic64_add(cryptlen, &alg->encrypt_tlen);
}
#endif
}
-static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
+static inline void crypto_stat_skcipher_decrypt(unsigned int cryptlen,
int ret, struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
@@ -507,7 +507,7 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
atomic64_inc(&alg->cipher_err_cnt);
} else {
atomic64_inc(&alg->decrypt_cnt);
- atomic64_add(req->cryptlen, &alg->decrypt_tlen);
+ atomic64_add(cryptlen, &alg->decrypt_tlen);
}
#endif
}
@@ -526,13 +526,15 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int cryptlen = req->cryptlen;
int ret;
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = tfm->encrypt(req);
- crypto_stat_skcipher_encrypt(req, ret, tfm->base.__crt_alg);
+ crypto_stat_skcipher_encrypt(cryptlen, ret, alg);
return ret;
}
@@ -550,13 +552,15 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
static inline int crypto_skcipher_decrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int cryptlen = req->cryptlen;
int ret;
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = tfm->decrypt(req);
- crypto_stat_skcipher_decrypt(req, ret, tfm->base.__crt_alg);
+ crypto_stat_skcipher_decrypt(cryptlen, ret, alg);
return ret;
}
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 7d913330402e..b9ab028ce862 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -975,34 +975,28 @@ static inline struct crypto_ablkcipher *crypto_ablkcipher_reqtfm(
return __crypto_ablkcipher_cast(req->base.tfm);
}
-static inline void crypto_stat_ablkcipher_encrypt(struct ablkcipher_request *req,
- int ret)
+static inline void crypto_stat_ablkcipher_encrypt(unsigned int nbytes, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct ablkcipher_tfm *crt =
- crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
+ atomic64_inc(&alg->cipher_err_cnt);
} else {
- atomic64_inc(&crt->base->base.__crt_alg->encrypt_cnt);
- atomic64_add(req->nbytes, &crt->base->base.__crt_alg->encrypt_tlen);
+ atomic64_inc(&alg->encrypt_cnt);
+ atomic64_add(nbytes, &alg->encrypt_tlen);
}
#endif
}
-static inline void crypto_stat_ablkcipher_decrypt(struct ablkcipher_request *req,
- int ret)
+static inline void crypto_stat_ablkcipher_decrypt(unsigned int nbytes, int ret,
+ struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
- struct ablkcipher_tfm *crt =
- crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
-
if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
- atomic64_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
+ atomic64_inc(&alg->cipher_err_cnt);
} else {
- atomic64_inc(&crt->base->base.__crt_alg->decrypt_cnt);
- atomic64_add(req->nbytes, &crt->base->base.__crt_alg->decrypt_tlen);
+ atomic64_inc(&alg->decrypt_cnt);
+ atomic64_add(nbytes, &alg->decrypt_tlen);
}
#endif
}
@@ -1022,10 +1016,12 @@ static inline int crypto_ablkcipher_encrypt(struct ablkcipher_request *req)
{
struct ablkcipher_tfm *crt =
crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
+ struct crypto_alg *alg = crt->base->base.__crt_alg;
+ unsigned int nbytes = req->nbytes;
int ret;
ret = crt->encrypt(req);
- crypto_stat_ablkcipher_encrypt(req, ret);
+ crypto_stat_ablkcipher_encrypt(nbytes, ret, alg);
return ret;
}
@@ -1044,10 +1040,12 @@ static inline int crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
{
struct ablkcipher_tfm *crt =
crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
+ struct crypto_alg *alg = crt->base->base.__crt_alg;
+ unsigned int nbytes = req->nbytes;
int ret;
ret = crt->decrypt(req);
- crypto_stat_ablkcipher_decrypt(req, ret);
+ crypto_stat_ablkcipher_decrypt(nbytes, ret, alg);
return ret;
}
--
2.18.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional
2018-11-05 12:51 ` [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional Corentin Labbe
@ 2018-11-06 1:41 ` Eric Biggers
2018-11-07 18:55 ` LABBE Corentin
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2018-11-06 1:41 UTC (permalink / raw)
To: Corentin Labbe; +Cc: davem, herbert, linux-crypto, linux-kernel
Hi Corentin,
On Mon, Nov 05, 2018 at 12:51:10PM +0000, Corentin Labbe wrote:
> Even if CRYPTO_STATS is set to n, some part of CRYPTO_STATS are
> compiled.
> This patch made all part of crypto_user_stat uncompiled in that case.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> crypto/Makefile | 3 ++-
> crypto/algapi.c | 2 ++
> include/crypto/internal/cryptouser.h | 17 +++++++++++++++++
> include/linux/crypto.h | 2 ++
> 4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 5c207c76abf7..1e9e5960946a 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -54,7 +54,8 @@ cryptomgr-y := algboss.o testmgr.o
>
> obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
> obj-$(CONFIG_CRYPTO_USER) += crypto_user.o
> -crypto_user-y := crypto_user_base.o crypto_user_stat.o
> +crypto_user-y := crypto_user_base.o
> +crypto_user-$(CONFIG_CRYPTO_STATS) += crypto_user_stat.o
> obj-$(CONFIG_CRYPTO_CMAC) += cmac.o
> obj-$(CONFIG_CRYPTO_HMAC) += hmac.o
> obj-$(CONFIG_CRYPTO_VMAC) += vmac.o
Doesn't CONFIG_CRYPTO_STATS also need to depend on CONFIG_CRYPTO_USER?
Currently you can enable it but without CRYPTO_USER the API won't be available.
config CRYPTO_STATS
bool "Crypto usage statistics for User-space"
should be:
config CRYPTO_STATS
bool "Crypto usage statistics for User-space"
depends on CRYPTO_USER
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index 3634ad6fe202..35de61d99cd5 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -515,6 +515,7 @@ struct crypto_alg {
>
> struct module *cra_module;
>
> +#ifdef CONFIG_CRYPTO_STATS
> union {
> atomic_t encrypt_cnt;
> atomic_t compress_cnt;
> @@ -552,6 +553,7 @@ struct crypto_alg {
> atomic_t compute_shared_secret_cnt;
> };
> atomic_t sign_cnt;
> +#endif
This is a long ifdef, so use:
#endif /* CONFIG_CRYPTO_STATS */
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64
2018-11-05 12:51 ` [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64 Corentin Labbe
@ 2018-11-06 1:42 ` Eric Biggers
2018-11-07 18:58 ` LABBE Corentin
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2018-11-06 1:42 UTC (permalink / raw)
To: Corentin Labbe; +Cc: davem, herbert, linux-crypto, linux-kernel
Hi Corentin,
On Mon, Nov 05, 2018 at 12:51:11PM +0000, Corentin Labbe wrote:
> All the 32-bit fields need to be 64-bit. In some cases, UINT32_MAX crypto
> operations can be done in seconds.
>
> Reported-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> crypto/algapi.c | 10 +--
> crypto/crypto_user_stat.c | 114 +++++++++++++++-----------------
> include/crypto/acompress.h | 8 +--
> include/crypto/aead.h | 8 +--
> include/crypto/akcipher.h | 16 ++---
> include/crypto/hash.h | 6 +-
> include/crypto/kpp.h | 12 ++--
> include/crypto/rng.h | 8 +--
> include/crypto/skcipher.h | 8 +--
> include/linux/crypto.h | 46 ++++++-------
> include/uapi/linux/cryptouser.h | 38 +++++------
> 11 files changed, 133 insertions(+), 141 deletions(-)
>
> diff --git a/crypto/algapi.c b/crypto/algapi.c
> index f5396c88e8cd..42fe316f80ee 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -259,13 +259,13 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
> list_add(&larval->alg.cra_list, &crypto_alg_list);
>
> #ifdef CONFIG_CRYPTO_STATS
> - atomic_set(&alg->encrypt_cnt, 0);
> - atomic_set(&alg->decrypt_cnt, 0);
> + atomic64_set(&alg->encrypt_cnt, 0);
> + atomic64_set(&alg->decrypt_cnt, 0);
> atomic64_set(&alg->encrypt_tlen, 0);
> atomic64_set(&alg->decrypt_tlen, 0);
> - atomic_set(&alg->verify_cnt, 0);
> - atomic_set(&alg->cipher_err_cnt, 0);
> - atomic_set(&alg->sign_cnt, 0);
> + atomic64_set(&alg->verify_cnt, 0);
> + atomic64_set(&alg->cipher_err_cnt, 0);
> + atomic64_set(&alg->sign_cnt, 0);
> #endif
>
> out:
> diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
> index a6fb2e6f618d..352569f378a0 100644
> --- a/crypto/crypto_user_stat.c
> +++ b/crypto/crypto_user_stat.c
> @@ -35,22 +35,21 @@ static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
> {
> struct crypto_stat raead;
> u64 v64;
> - u32 v32;
>
> memset(&raead, 0, sizeof(raead));
>
> strscpy(raead.type, "aead", sizeof(raead.type));
>
> - v32 = atomic_read(&alg->encrypt_cnt);
> - raead.stat_encrypt_cnt = v32;
> + v64 = atomic64_read(&alg->encrypt_cnt);
> + raead.stat_encrypt_cnt = v64;
> v64 = atomic64_read(&alg->encrypt_tlen);
> raead.stat_encrypt_tlen = v64;
> - v32 = atomic_read(&alg->decrypt_cnt);
> - raead.stat_decrypt_cnt = v32;
> + v64 = atomic64_read(&alg->decrypt_cnt);
> + raead.stat_decrypt_cnt = v64;
> v64 = atomic64_read(&alg->decrypt_tlen);
> raead.stat_decrypt_tlen = v64;
> - v32 = atomic_read(&alg->aead_err_cnt);
> - raead.stat_aead_err_cnt = v32;
> + v64 = atomic64_read(&alg->aead_err_cnt);
> + raead.stat_aead_err_cnt = v64;
>
> return nla_put(skb, CRYPTOCFGA_STAT_AEAD, sizeof(raead), &raead);
> }
Why not assign the result of atomic64_read() directly?
I don't see the point of the 'v64' variable.
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures
2018-11-05 12:51 ` [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures Corentin Labbe
@ 2018-11-06 1:44 ` Eric Biggers
2018-11-07 19:13 ` LABBE Corentin
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2018-11-06 1:44 UTC (permalink / raw)
To: Corentin Labbe; +Cc: davem, herbert, linux-crypto, linux-kernel
Hi Corentin,
On Mon, Nov 05, 2018 at 12:51:12PM +0000, Corentin Labbe wrote:
> It is cleaner to have each stat in their own structures.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> crypto/crypto_user_stat.c | 20 +++----
> include/uapi/linux/cryptouser.h | 101 ++++++++++++++++++++------------
> 2 files changed, 73 insertions(+), 48 deletions(-)
>
> diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
> index 352569f378a0..3c14be2f7a1b 100644
> --- a/crypto/crypto_user_stat.c
> +++ b/crypto/crypto_user_stat.c
> @@ -33,7 +33,7 @@ struct crypto_dump_info {
>
> static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat raead;
> + struct crypto_stat_aead raead;
> u64 v64;
>
> memset(&raead, 0, sizeof(raead));
> @@ -56,7 +56,7 @@ static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rcipher;
> + struct crypto_stat_cipher rcipher;
> u64 v64;
>
> memset(&rcipher, 0, sizeof(rcipher));
> @@ -79,7 +79,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rcomp;
> + struct crypto_stat_compress rcomp;
> u64 v64;
>
> memset(&rcomp, 0, sizeof(rcomp));
> @@ -101,7 +101,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat racomp;
> + struct crypto_stat_compress racomp;
> u64 v64;
>
> memset(&racomp, 0, sizeof(racomp));
> @@ -123,7 +123,7 @@ static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rakcipher;
> + struct crypto_stat_akcipher rakcipher;
> u64 v64;
>
> memset(&rakcipher, 0, sizeof(rakcipher));
> @@ -150,7 +150,7 @@ static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rkpp;
> + struct crypto_stat_kpp rkpp;
> u64 v;
>
> memset(&rkpp, 0, sizeof(rkpp));
> @@ -171,7 +171,7 @@ static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rhash;
> + struct crypto_stat_hash rhash;
> u64 v64;
>
> memset(&rhash, 0, sizeof(rhash));
> @@ -190,7 +190,7 @@ static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rhash;
> + struct crypto_stat_hash rhash;
> u64 v64;
>
> memset(&rhash, 0, sizeof(rhash));
> @@ -209,7 +209,7 @@ static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
>
> static int crypto_report_rng(struct sk_buff *skb, struct crypto_alg *alg)
> {
> - struct crypto_stat rrng;
> + struct crypto_stat_rng rrng;
> u64 v64;
>
> memset(&rrng, 0, sizeof(rrng));
> @@ -248,7 +248,7 @@ static int crypto_reportstat_one(struct crypto_alg *alg,
> if (nla_put_u32(skb, CRYPTOCFGA_PRIORITY_VAL, alg->cra_priority))
> goto nla_put_failure;
> if (alg->cra_flags & CRYPTO_ALG_LARVAL) {
> - struct crypto_stat rl;
> + struct crypto_stat_larval rl;
>
> memset(&rl, 0, sizeof(rl));
> strscpy(rl.type, "larval", sizeof(rl.type));
> diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
> index 9f8187077ce4..790b5c6511e5 100644
> --- a/include/uapi/linux/cryptouser.h
> +++ b/include/uapi/linux/cryptouser.h
> @@ -76,47 +76,72 @@ struct crypto_user_alg {
> __u32 cru_flags;
> };
>
> -struct crypto_stat {
> - char type[CRYPTO_MAX_NAME];
> - union {
> - __u64 stat_encrypt_cnt;
> - __u64 stat_compress_cnt;
> - __u64 stat_generate_cnt;
> - __u64 stat_hash_cnt;
> - __u64 stat_setsecret_cnt;
> - };
> - union {
> - __u64 stat_encrypt_tlen;
> - __u64 stat_compress_tlen;
> - __u64 stat_generate_tlen;
> - __u64 stat_hash_tlen;
> - };
> - union {
> - __u64 stat_akcipher_err_cnt;
> - __u64 stat_cipher_err_cnt;
> - __u64 stat_compress_err_cnt;
> - __u64 stat_aead_err_cnt;
> - __u64 stat_hash_err_cnt;
> - __u64 stat_rng_err_cnt;
> - __u64 stat_kpp_err_cnt;
> - };
> - union {
> - __u64 stat_decrypt_cnt;
> - __u64 stat_decompress_cnt;
> - __u64 stat_seed_cnt;
> - __u64 stat_generate_public_key_cnt;
> - };
> - union {
> - __u64 stat_decrypt_tlen;
> - __u64 stat_decompress_tlen;
> - };
> - union {
> - __u64 stat_verify_cnt;
> - __u64 stat_compute_shared_secret_cnt;
> - };
> +struct crypto_stat_cipher {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_encrypt_cnt;
> + __u64 stat_encrypt_tlen;
> + __u64 stat_decrypt_cnt;
> + __u64 stat_decrypt_tlen;
> + __u64 stat_cipher_err_cnt;
> +};
> +
> +struct crypto_stat_aead {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_encrypt_cnt;
> + __u64 stat_encrypt_tlen;
> + __u64 stat_decrypt_cnt;
> + __u64 stat_decrypt_tlen;
> + __u64 stat_cipher_err_cnt;
> + __u64 stat_aead_err_cnt;
> +};
> +
> +struct crypto_stat_akcipher {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_encrypt_cnt;
> + __u64 stat_encrypt_tlen;
> + __u64 stat_decrypt_cnt;
> + __u64 stat_decrypt_tlen;
> + __u64 stat_akcipher_err_cnt;
> + __u64 stat_verify_cnt;
> __u64 stat_sign_cnt;
> };
>
> +struct crypto_stat_compress {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_compress_cnt;
> + __u64 stat_compress_tlen;
> + __u64 stat_decompress_cnt;
> + __u64 stat_decompress_tlen;
> + __u64 stat_compress_err_cnt;
> +};
> +
> +struct crypto_stat_rng {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_generate_cnt;
> + __u64 stat_generate_tlen;
> + __u64 stat_rng_err_cnt;
> + __u64 stat_seed_cnt;
> +};
> +
> +struct crypto_stat_hash {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_hash_cnt;
> + __u64 stat_hash_tlen;
> + __u64 stat_hash_err_cnt;
> +};
> +
> +struct crypto_stat_kpp {
> + char type[CRYPTO_MAX_NAME];
> + __u64 stat_setsecret_cnt;
> + __u64 stat_kpp_err_cnt;
> + __u64 stat_generate_public_key_cnt;
> + __u64 stat_compute_shared_secret_cnt;
> +};
> +
> +struct crypto_stat_larval {
> + char type[CRYPTO_MAX_NAME];
> +};
> +
> struct crypto_report_larval {
> char type[CRYPTO_MAX_NAME];
> };
> --
> 2.18.1
>
Is there any particular reason this patch only changes the UAPI structures,
not the internal structures in 'struct crypto_alg'?
- Eric
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request
2018-11-05 12:51 ` [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request Corentin Labbe
@ 2018-11-06 1:49 ` Eric Biggers
2018-11-07 19:34 ` LABBE Corentin
0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2018-11-06 1:49 UTC (permalink / raw)
To: Corentin Labbe; +Cc: davem, herbert, linux-crypto, linux-kernel
Hi Corentin,
On Mon, Nov 05, 2018 at 12:51:14PM +0000, Corentin Labbe wrote:
> All crypto_stats functions use the struct xxx_request for feeding stats,
> but in some case this structure could already be freed.
>
> For fixing this, the needed parameters (len and alg) will be stored
> before the request being executed.
> Fixes: cac5818c25d0 ("crypto: user - Implement a generic crypto statistics")
> Reported-by: syzbot <syzbot+6939a606a5305e9e9799@syzkaller.appspotmail.com>
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> crypto/ahash.c | 14 ++++++++--
> include/crypto/acompress.h | 30 ++++++++++----------
> include/crypto/aead.h | 30 ++++++++++----------
> include/crypto/akcipher.h | 56 ++++++++++++++++++--------------------
> include/crypto/hash.h | 25 +++++++++--------
> include/crypto/kpp.h | 22 +++++++--------
> include/crypto/skcipher.h | 16 +++++++----
> include/linux/crypto.h | 34 +++++++++++------------
> 8 files changed, 118 insertions(+), 109 deletions(-)
>
> diff --git a/crypto/ahash.c b/crypto/ahash.c
> index 3a348fbcf8f9..3f4c44c1e80f 100644
> --- a/crypto/ahash.c
> +++ b/crypto/ahash.c
> @@ -364,20 +364,26 @@ static int crypto_ahash_op(struct ahash_request *req,
>
> int crypto_ahash_final(struct ahash_request *req)
> {
> + struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int nbytes = req->nbytes;
> int ret;
>
> ret = crypto_ahash_op(req, crypto_ahash_reqtfm(req)->final);
> - crypto_stat_ahash_final(req, ret);
> + crypto_stat_ahash_final(nbytes, ret, alg);
> return ret;
> }
> EXPORT_SYMBOL_GPL(crypto_ahash_final);
I'm not confident this is enough. If the request is being processed
asynchronously, the completion callback could be used to signal another thread
to go ahead and free the resources, including not only the request but also the
'tfm', which could even result in the last reference to the 'alg' being dropped
and therefore the 'alg' being freed. If I'm wrong, then please explain why :-)
Note: I'd also prefer a solution that is more obviously zero-overhead in the
!CONFIG_CRYPTO_STATS case.
- Eric
>
> int crypto_ahash_finup(struct ahash_request *req)
> {
> + struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int nbytes = req->nbytes;
> int ret;
>
> ret = crypto_ahash_op(req, crypto_ahash_reqtfm(req)->finup);
> - crypto_stat_ahash_final(req, ret);
> + crypto_stat_ahash_final(nbytes, ret, alg);
> return ret;
> }
> EXPORT_SYMBOL_GPL(crypto_ahash_finup);
> @@ -385,13 +391,15 @@ EXPORT_SYMBOL_GPL(crypto_ahash_finup);
> int crypto_ahash_digest(struct ahash_request *req)
> {
> struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int nbytes = req->nbytes;
> int ret;
>
> if (crypto_ahash_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
> ret = -ENOKEY;
> else
> ret = crypto_ahash_op(req, tfm->digest);
> - crypto_stat_ahash_final(req, ret);
> + crypto_stat_ahash_final(nbytes, ret, alg);
> return ret;
> }
> EXPORT_SYMBOL_GPL(crypto_ahash_digest);
> diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
> index f79918196811..396407cc34c7 100644
> --- a/include/crypto/acompress.h
> +++ b/include/crypto/acompress.h
> @@ -234,30 +234,28 @@ static inline void acomp_request_set_params(struct acomp_req *req,
> req->flags |= CRYPTO_ACOMP_ALLOC_OUTPUT;
> }
>
> -static inline void crypto_stat_compress(struct acomp_req *req, int ret)
> +static inline void crypto_stat_compress(unsigned int slen, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->compress_err_cnt);
> + atomic64_inc(&alg->compress_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->compress_cnt);
> - atomic64_add(req->slen, &tfm->base.__crt_alg->compress_tlen);
> + atomic64_inc(&alg->compress_cnt);
> + atomic64_add(slen, &alg->compress_tlen);
> }
> #endif
> }
>
> -static inline void crypto_stat_decompress(struct acomp_req *req, int ret)
> +static inline void crypto_stat_decompress(unsigned int slen, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->compress_err_cnt);
> + atomic64_inc(&alg->compress_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->decompress_cnt);
> - atomic64_add(req->slen, &tfm->base.__crt_alg->decompress_tlen);
> + atomic64_inc(&alg->decompress_cnt);
> + atomic64_add(slen, &alg->decompress_tlen);
> }
> #endif
> }
> @@ -274,10 +272,12 @@ static inline void crypto_stat_decompress(struct acomp_req *req, int ret)
> static inline int crypto_acomp_compress(struct acomp_req *req)
> {
> struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int slen = req->slen;
> int ret;
>
> ret = tfm->compress(req);
> - crypto_stat_compress(req, ret);
> + crypto_stat_compress(slen, ret, alg);
> return ret;
> }
>
> @@ -293,10 +293,12 @@ static inline int crypto_acomp_compress(struct acomp_req *req)
> static inline int crypto_acomp_decompress(struct acomp_req *req)
> {
> struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int slen = req->slen;
> int ret;
>
> ret = tfm->decompress(req);
> - crypto_stat_decompress(req, ret);
> + crypto_stat_decompress(slen, ret, alg);
> return ret;
> }
>
> diff --git a/include/crypto/aead.h b/include/crypto/aead.h
> index 99afd78c665d..c959cd39049d 100644
> --- a/include/crypto/aead.h
> +++ b/include/crypto/aead.h
> @@ -306,30 +306,28 @@ static inline struct crypto_aead *crypto_aead_reqtfm(struct aead_request *req)
> return __crypto_aead_cast(req->base.tfm);
> }
>
> -static inline void crypto_stat_aead_encrypt(struct aead_request *req, int ret)
> +static inline void crypto_stat_aead_encrypt(unsigned int cryptlen,
> + struct crypto_alg *alg, int ret)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->aead_err_cnt);
> + atomic64_inc(&alg->aead_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->encrypt_cnt);
> - atomic64_add(req->cryptlen, &tfm->base.__crt_alg->encrypt_tlen);
> + atomic64_inc(&alg->encrypt_cnt);
> + atomic64_add(cryptlen, &alg->encrypt_tlen);
> }
> #endif
> }
>
> -static inline void crypto_stat_aead_decrypt(struct aead_request *req, int ret)
> +static inline void crypto_stat_aead_decrypt(unsigned int cryptlen,
> + struct crypto_alg *alg, int ret)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->aead_err_cnt);
> + atomic64_inc(&alg->aead_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->decrypt_cnt);
> - atomic64_add(req->cryptlen, &tfm->base.__crt_alg->decrypt_tlen);
> + atomic64_inc(&alg->decrypt_cnt);
> + atomic64_add(cryptlen, &alg->decrypt_tlen);
> }
> #endif
> }
> @@ -356,13 +354,15 @@ static inline void crypto_stat_aead_decrypt(struct aead_request *req, int ret)
> static inline int crypto_aead_encrypt(struct aead_request *req)
> {
> struct crypto_aead *aead = crypto_aead_reqtfm(req);
> + struct crypto_alg *alg = aead->base.__crt_alg;
> + unsigned int cryptlen = req->cryptlen;
> int ret;
>
> if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
> ret = -ENOKEY;
> else
> ret = crypto_aead_alg(aead)->encrypt(req);
> - crypto_stat_aead_encrypt(req, ret);
> + crypto_stat_aead_encrypt(cryptlen, alg, ret);
> return ret;
> }
>
> @@ -391,6 +391,8 @@ static inline int crypto_aead_encrypt(struct aead_request *req)
> static inline int crypto_aead_decrypt(struct aead_request *req)
> {
> struct crypto_aead *aead = crypto_aead_reqtfm(req);
> + struct crypto_alg *alg = aead->base.__crt_alg;
> + unsigned int cryptlen = req->cryptlen;
> int ret;
>
> if (crypto_aead_get_flags(aead) & CRYPTO_TFM_NEED_KEY)
> @@ -399,7 +401,7 @@ static inline int crypto_aead_decrypt(struct aead_request *req)
> ret = -EINVAL;
> else
> ret = crypto_aead_alg(aead)->decrypt(req);
> - crypto_stat_aead_decrypt(req, ret);
> + crypto_stat_aead_decrypt(cryptlen, alg, ret);
> return ret;
> }
>
> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> index 97056fd5e718..f20cdb775674 100644
> --- a/include/crypto/akcipher.h
> +++ b/include/crypto/akcipher.h
> @@ -271,59 +271,49 @@ static inline unsigned int crypto_akcipher_maxsize(struct crypto_akcipher *tfm)
> return alg->max_size(tfm);
> }
>
> -static inline void crypto_stat_akcipher_encrypt(struct akcipher_request *req,
> - int ret)
> +static inline void crypto_stat_akcipher_encrypt(unsigned int src_len, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
> + atomic64_inc(&alg->akcipher_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->encrypt_cnt);
> - atomic64_add(req->src_len, &tfm->base.__crt_alg->encrypt_tlen);
> + atomic64_inc(&alg->encrypt_cnt);
> + atomic64_add(src_len, &alg->encrypt_tlen);
> }
> #endif
> }
>
> -static inline void crypto_stat_akcipher_decrypt(struct akcipher_request *req,
> - int ret)
> +static inline void crypto_stat_akcipher_decrypt(unsigned int src_len, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
> + atomic64_inc(&alg->akcipher_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->decrypt_cnt);
> - atomic64_add(req->src_len, &tfm->base.__crt_alg->decrypt_tlen);
> + atomic64_inc(&alg->decrypt_cnt);
> + atomic64_add(src_len, &alg->decrypt_tlen);
> }
> #endif
> }
>
> -static inline void crypto_stat_akcipher_sign(struct akcipher_request *req,
> - int ret)
> +static inline void crypto_stat_akcipher_sign(int ret, struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY)
> - atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
> + atomic64_inc(&alg->akcipher_err_cnt);
> else
> - atomic64_inc(&tfm->base.__crt_alg->sign_cnt);
> + atomic64_inc(&alg->sign_cnt);
> #endif
> }
>
> -static inline void crypto_stat_akcipher_verify(struct akcipher_request *req,
> - int ret)
> +static inline void crypto_stat_akcipher_verify(int ret, struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY)
> - atomic64_inc(&tfm->base.__crt_alg->akcipher_err_cnt);
> + atomic64_inc(&alg->akcipher_err_cnt);
> else
> - atomic64_inc(&tfm->base.__crt_alg->verify_cnt);
> + atomic64_inc(&alg->verify_cnt);
> #endif
> }
>
> @@ -341,10 +331,12 @@ static inline int crypto_akcipher_encrypt(struct akcipher_request *req)
> {
> struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> + struct crypto_alg *calg = tfm->base.__crt_alg;
> + unsigned int src_len = req->src_len;
> int ret;
>
> ret = alg->encrypt(req);
> - crypto_stat_akcipher_encrypt(req, ret);
> + crypto_stat_akcipher_encrypt(src_len, ret, calg);
> return ret;
> }
>
> @@ -362,10 +354,12 @@ static inline int crypto_akcipher_decrypt(struct akcipher_request *req)
> {
> struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> + struct crypto_alg *calg = tfm->base.__crt_alg;
> + unsigned int src_len = req->src_len;
> int ret;
>
> ret = alg->decrypt(req);
> - crypto_stat_akcipher_decrypt(req, ret);
> + crypto_stat_akcipher_decrypt(src_len, ret, calg);
> return ret;
> }
>
> @@ -383,10 +377,11 @@ static inline int crypto_akcipher_sign(struct akcipher_request *req)
> {
> struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> + struct crypto_alg *calg = tfm->base.__crt_alg;
> int ret;
>
> ret = alg->sign(req);
> - crypto_stat_akcipher_sign(req, ret);
> + crypto_stat_akcipher_sign(ret, calg);
> return ret;
> }
>
> @@ -404,10 +399,11 @@ static inline int crypto_akcipher_verify(struct akcipher_request *req)
> {
> struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> + struct crypto_alg *calg = tfm->base.__crt_alg;
> int ret;
>
> ret = alg->verify(req);
> - crypto_stat_akcipher_verify(req, ret);
> + crypto_stat_akcipher_verify(ret, calg);
> return ret;
> }
>
> diff --git a/include/crypto/hash.h b/include/crypto/hash.h
> index 52920bed05ba..83fe47f34db1 100644
> --- a/include/crypto/hash.h
> +++ b/include/crypto/hash.h
> @@ -412,28 +412,26 @@ static inline void *ahash_request_ctx(struct ahash_request *req)
> int crypto_ahash_setkey(struct crypto_ahash *tfm, const u8 *key,
> unsigned int keylen);
>
> -static inline void crypto_stat_ahash_update(struct ahash_request *req, int ret)
> +static inline void crypto_stat_ahash_update(unsigned int nbytes, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY)
> - atomic64_inc(&tfm->base.__crt_alg->hash_err_cnt);
> + atomic64_inc(&alg->hash_err_cnt);
> else
> - atomic64_add(req->nbytes, &tfm->base.__crt_alg->hash_tlen);
> + atomic64_add(nbytes, &alg->hash_tlen);
> #endif
> }
>
> -static inline void crypto_stat_ahash_final(struct ahash_request *req, int ret)
> +static inline void crypto_stat_ahash_final(unsigned int nbytes, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&tfm->base.__crt_alg->hash_err_cnt);
> + atomic64_inc(&alg->hash_err_cnt);
> } else {
> - atomic64_inc(&tfm->base.__crt_alg->hash_cnt);
> - atomic64_add(req->nbytes, &tfm->base.__crt_alg->hash_tlen);
> + atomic64_inc(&alg->hash_cnt);
> + atomic64_add(nbytes, &alg->hash_tlen);
> }
> #endif
> }
> @@ -552,10 +550,13 @@ static inline int crypto_ahash_init(struct ahash_request *req)
> */
> static inline int crypto_ahash_update(struct ahash_request *req)
> {
> + struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int nbytes = req->nbytes;
> int ret;
>
> ret = crypto_ahash_reqtfm(req)->update(req);
> - crypto_stat_ahash_update(req, ret);
> + crypto_stat_ahash_update(nbytes, ret, alg);
> return ret;
> }
>
> diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
> index bd5103a80919..525689505b49 100644
> --- a/include/crypto/kpp.h
> +++ b/include/crypto/kpp.h
> @@ -278,29 +278,25 @@ static inline void crypto_stat_kpp_set_secret(struct crypto_kpp *tfm, int ret)
> #endif
> }
>
> -static inline void crypto_stat_kpp_generate_public_key(struct kpp_request *req,
> +static inline void crypto_stat_kpp_generate_public_key(struct crypto_alg *alg,
> int ret)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
> -
> if (ret)
> - atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
> + atomic64_inc(&alg->kpp_err_cnt);
> else
> - atomic64_inc(&tfm->base.__crt_alg->generate_public_key_cnt);
> + atomic64_inc(&alg->generate_public_key_cnt);
> #endif
> }
>
> -static inline void crypto_stat_kpp_compute_shared_secret(struct kpp_request *req,
> +static inline void crypto_stat_kpp_compute_shared_secret(struct crypto_alg *alg,
> int ret)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
> -
> if (ret)
> - atomic64_inc(&tfm->base.__crt_alg->kpp_err_cnt);
> + atomic64_inc(&alg->kpp_err_cnt);
> else
> - atomic64_inc(&tfm->base.__crt_alg->compute_shared_secret_cnt);
> + atomic64_inc(&alg->compute_shared_secret_cnt);
> #endif
> }
>
> @@ -347,10 +343,11 @@ static inline int crypto_kpp_generate_public_key(struct kpp_request *req)
> {
> struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
> struct kpp_alg *alg = crypto_kpp_alg(tfm);
> + struct crypto_alg *calg = tfm->base.__crt_alg;
> int ret;
>
> ret = alg->generate_public_key(req);
> - crypto_stat_kpp_generate_public_key(req, ret);
> + crypto_stat_kpp_generate_public_key(calg, ret);
> return ret;
> }
>
> @@ -368,10 +365,11 @@ static inline int crypto_kpp_compute_shared_secret(struct kpp_request *req)
> {
> struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
> struct kpp_alg *alg = crypto_kpp_alg(tfm);
> + struct crypto_alg *calg = tfm->base.__crt_alg;
> int ret;
>
> ret = alg->compute_shared_secret(req);
> - crypto_stat_kpp_compute_shared_secret(req, ret);
> + crypto_stat_kpp_compute_shared_secret(calg, ret);
> return ret;
> }
>
> diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
> index dff54731ddf4..a68b227de96c 100644
> --- a/include/crypto/skcipher.h
> +++ b/include/crypto/skcipher.h
> @@ -486,7 +486,7 @@ static inline struct crypto_sync_skcipher *crypto_sync_skcipher_reqtfm(
> return container_of(tfm, struct crypto_sync_skcipher, base);
> }
>
> -static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
> +static inline void crypto_stat_skcipher_encrypt(unsigned int cryptlen,
> int ret, struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> @@ -494,12 +494,12 @@ static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
> atomic64_inc(&alg->cipher_err_cnt);
> } else {
> atomic64_inc(&alg->encrypt_cnt);
> - atomic64_add(req->cryptlen, &alg->encrypt_tlen);
> + atomic64_add(cryptlen, &alg->encrypt_tlen);
> }
> #endif
> }
>
> -static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
> +static inline void crypto_stat_skcipher_decrypt(unsigned int cryptlen,
> int ret, struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> @@ -507,7 +507,7 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
> atomic64_inc(&alg->cipher_err_cnt);
> } else {
> atomic64_inc(&alg->decrypt_cnt);
> - atomic64_add(req->cryptlen, &alg->decrypt_tlen);
> + atomic64_add(cryptlen, &alg->decrypt_tlen);
> }
> #endif
> }
> @@ -526,13 +526,15 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
> static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
> {
> struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int cryptlen = req->cryptlen;
> int ret;
>
> if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
> ret = -ENOKEY;
> else
> ret = tfm->encrypt(req);
> - crypto_stat_skcipher_encrypt(req, ret, tfm->base.__crt_alg);
> + crypto_stat_skcipher_encrypt(cryptlen, ret, alg);
> return ret;
> }
>
> @@ -550,13 +552,15 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
> static inline int crypto_skcipher_decrypt(struct skcipher_request *req)
> {
> struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> + struct crypto_alg *alg = tfm->base.__crt_alg;
> + unsigned int cryptlen = req->cryptlen;
> int ret;
>
> if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
> ret = -ENOKEY;
> else
> ret = tfm->decrypt(req);
> - crypto_stat_skcipher_decrypt(req, ret, tfm->base.__crt_alg);
> + crypto_stat_skcipher_decrypt(cryptlen, ret, alg);
> return ret;
> }
>
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index 7d913330402e..b9ab028ce862 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -975,34 +975,28 @@ static inline struct crypto_ablkcipher *crypto_ablkcipher_reqtfm(
> return __crypto_ablkcipher_cast(req->base.tfm);
> }
>
> -static inline void crypto_stat_ablkcipher_encrypt(struct ablkcipher_request *req,
> - int ret)
> +static inline void crypto_stat_ablkcipher_encrypt(unsigned int nbytes, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct ablkcipher_tfm *crt =
> - crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
> + atomic64_inc(&alg->cipher_err_cnt);
> } else {
> - atomic64_inc(&crt->base->base.__crt_alg->encrypt_cnt);
> - atomic64_add(req->nbytes, &crt->base->base.__crt_alg->encrypt_tlen);
> + atomic64_inc(&alg->encrypt_cnt);
> + atomic64_add(nbytes, &alg->encrypt_tlen);
> }
> #endif
> }
>
> -static inline void crypto_stat_ablkcipher_decrypt(struct ablkcipher_request *req,
> - int ret)
> +static inline void crypto_stat_ablkcipher_decrypt(unsigned int nbytes, int ret,
> + struct crypto_alg *alg)
> {
> #ifdef CONFIG_CRYPTO_STATS
> - struct ablkcipher_tfm *crt =
> - crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
> -
> if (ret && ret != -EINPROGRESS && ret != -EBUSY) {
> - atomic64_inc(&crt->base->base.__crt_alg->cipher_err_cnt);
> + atomic64_inc(&alg->cipher_err_cnt);
> } else {
> - atomic64_inc(&crt->base->base.__crt_alg->decrypt_cnt);
> - atomic64_add(req->nbytes, &crt->base->base.__crt_alg->decrypt_tlen);
> + atomic64_inc(&alg->decrypt_cnt);
> + atomic64_add(nbytes, &alg->decrypt_tlen);
> }
> #endif
> }
> @@ -1022,10 +1016,12 @@ static inline int crypto_ablkcipher_encrypt(struct ablkcipher_request *req)
> {
> struct ablkcipher_tfm *crt =
> crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
> + struct crypto_alg *alg = crt->base->base.__crt_alg;
> + unsigned int nbytes = req->nbytes;
> int ret;
>
> ret = crt->encrypt(req);
> - crypto_stat_ablkcipher_encrypt(req, ret);
> + crypto_stat_ablkcipher_encrypt(nbytes, ret, alg);
> return ret;
> }
>
> @@ -1044,10 +1040,12 @@ static inline int crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
> {
> struct ablkcipher_tfm *crt =
> crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
> + struct crypto_alg *alg = crt->base->base.__crt_alg;
> + unsigned int nbytes = req->nbytes;
> int ret;
>
> ret = crt->decrypt(req);
> - crypto_stat_ablkcipher_decrypt(req, ret);
> + crypto_stat_ablkcipher_decrypt(nbytes, ret, alg);
> return ret;
> }
>
> --
> 2.18.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional
2018-11-06 1:41 ` Eric Biggers
@ 2018-11-07 18:55 ` LABBE Corentin
0 siblings, 0 replies; 14+ messages in thread
From: LABBE Corentin @ 2018-11-07 18:55 UTC (permalink / raw)
To: Eric Biggers; +Cc: davem, herbert, linux-crypto, linux-kernel
On Mon, Nov 05, 2018 at 05:41:27PM -0800, Eric Biggers wrote:
> Hi Corentin,
>
> On Mon, Nov 05, 2018 at 12:51:10PM +0000, Corentin Labbe wrote:
> > Even if CRYPTO_STATS is set to n, some part of CRYPTO_STATS are
> > compiled.
> > This patch made all part of crypto_user_stat uncompiled in that case.
> >
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> > crypto/Makefile | 3 ++-
> > crypto/algapi.c | 2 ++
> > include/crypto/internal/cryptouser.h | 17 +++++++++++++++++
> > include/linux/crypto.h | 2 ++
> > 4 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/crypto/Makefile b/crypto/Makefile
> > index 5c207c76abf7..1e9e5960946a 100644
> > --- a/crypto/Makefile
> > +++ b/crypto/Makefile
> > @@ -54,7 +54,8 @@ cryptomgr-y := algboss.o testmgr.o
> >
> > obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
> > obj-$(CONFIG_CRYPTO_USER) += crypto_user.o
> > -crypto_user-y := crypto_user_base.o crypto_user_stat.o
> > +crypto_user-y := crypto_user_base.o
> > +crypto_user-$(CONFIG_CRYPTO_STATS) += crypto_user_stat.o
> > obj-$(CONFIG_CRYPTO_CMAC) += cmac.o
> > obj-$(CONFIG_CRYPTO_HMAC) += hmac.o
> > obj-$(CONFIG_CRYPTO_VMAC) += vmac.o
>
> Doesn't CONFIG_CRYPTO_STATS also need to depend on CONFIG_CRYPTO_USER?
> Currently you can enable it but without CRYPTO_USER the API won't be available.
>
> config CRYPTO_STATS
> bool "Crypto usage statistics for User-space"
>
> should be:
>
> config CRYPTO_STATS
> bool "Crypto usage statistics for User-space"
> depends on CRYPTO_USER
>
> > diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> > index 3634ad6fe202..35de61d99cd5 100644
> > --- a/include/linux/crypto.h
> > +++ b/include/linux/crypto.h
> > @@ -515,6 +515,7 @@ struct crypto_alg {
> >
> > struct module *cra_module;
> >
> > +#ifdef CONFIG_CRYPTO_STATS
> > union {
> > atomic_t encrypt_cnt;
> > atomic_t compress_cnt;
> > @@ -552,6 +553,7 @@ struct crypto_alg {
> > atomic_t compute_shared_secret_cnt;
> > };
> > atomic_t sign_cnt;
> > +#endif
>
> This is a long ifdef, so use:
>
> #endif /* CONFIG_CRYPTO_STATS */
>
> - Eric
I will fix that
Thanks
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64
2018-11-06 1:42 ` Eric Biggers
@ 2018-11-07 18:58 ` LABBE Corentin
0 siblings, 0 replies; 14+ messages in thread
From: LABBE Corentin @ 2018-11-07 18:58 UTC (permalink / raw)
To: Eric Biggers; +Cc: davem, herbert, linux-crypto, linux-kernel
On Mon, Nov 05, 2018 at 05:42:42PM -0800, Eric Biggers wrote:
> Hi Corentin,
>
> On Mon, Nov 05, 2018 at 12:51:11PM +0000, Corentin Labbe wrote:
> > All the 32-bit fields need to be 64-bit. In some cases, UINT32_MAX crypto
> > operations can be done in seconds.
> >
> > Reported-by: Eric Biggers <ebiggers@kernel.org>
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> > crypto/algapi.c | 10 +--
> > crypto/crypto_user_stat.c | 114 +++++++++++++++-----------------
> > include/crypto/acompress.h | 8 +--
> > include/crypto/aead.h | 8 +--
> > include/crypto/akcipher.h | 16 ++---
> > include/crypto/hash.h | 6 +-
> > include/crypto/kpp.h | 12 ++--
> > include/crypto/rng.h | 8 +--
> > include/crypto/skcipher.h | 8 +--
> > include/linux/crypto.h | 46 ++++++-------
> > include/uapi/linux/cryptouser.h | 38 +++++------
> > 11 files changed, 133 insertions(+), 141 deletions(-)
> >
> > diff --git a/crypto/algapi.c b/crypto/algapi.c
> > index f5396c88e8cd..42fe316f80ee 100644
> > --- a/crypto/algapi.c
> > +++ b/crypto/algapi.c
> > @@ -259,13 +259,13 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
> > list_add(&larval->alg.cra_list, &crypto_alg_list);
> >
> > #ifdef CONFIG_CRYPTO_STATS
> > - atomic_set(&alg->encrypt_cnt, 0);
> > - atomic_set(&alg->decrypt_cnt, 0);
> > + atomic64_set(&alg->encrypt_cnt, 0);
> > + atomic64_set(&alg->decrypt_cnt, 0);
> > atomic64_set(&alg->encrypt_tlen, 0);
> > atomic64_set(&alg->decrypt_tlen, 0);
> > - atomic_set(&alg->verify_cnt, 0);
> > - atomic_set(&alg->cipher_err_cnt, 0);
> > - atomic_set(&alg->sign_cnt, 0);
> > + atomic64_set(&alg->verify_cnt, 0);
> > + atomic64_set(&alg->cipher_err_cnt, 0);
> > + atomic64_set(&alg->sign_cnt, 0);
> > #endif
> >
> > out:
> > diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
> > index a6fb2e6f618d..352569f378a0 100644
> > --- a/crypto/crypto_user_stat.c
> > +++ b/crypto/crypto_user_stat.c
> > @@ -35,22 +35,21 @@ static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > struct crypto_stat raead;
> > u64 v64;
> > - u32 v32;
> >
> > memset(&raead, 0, sizeof(raead));
> >
> > strscpy(raead.type, "aead", sizeof(raead.type));
> >
> > - v32 = atomic_read(&alg->encrypt_cnt);
> > - raead.stat_encrypt_cnt = v32;
> > + v64 = atomic64_read(&alg->encrypt_cnt);
> > + raead.stat_encrypt_cnt = v64;
> > v64 = atomic64_read(&alg->encrypt_tlen);
> > raead.stat_encrypt_tlen = v64;
> > - v32 = atomic_read(&alg->decrypt_cnt);
> > - raead.stat_decrypt_cnt = v32;
> > + v64 = atomic64_read(&alg->decrypt_cnt);
> > + raead.stat_decrypt_cnt = v64;
> > v64 = atomic64_read(&alg->decrypt_tlen);
> > raead.stat_decrypt_tlen = v64;
> > - v32 = atomic_read(&alg->aead_err_cnt);
> > - raead.stat_aead_err_cnt = v32;
> > + v64 = atomic64_read(&alg->aead_err_cnt);
> > + raead.stat_aead_err_cnt = v64;
> >
> > return nla_put(skb, CRYPTOCFGA_STAT_AEAD, sizeof(raead), &raead);
> > }
>
> Why not assign the result of atomic64_read() directly?
> I don't see the point of the 'v64' variable.
>
Yes it will be more compact and easier to read
Thanks
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures
2018-11-06 1:44 ` Eric Biggers
@ 2018-11-07 19:13 ` LABBE Corentin
0 siblings, 0 replies; 14+ messages in thread
From: LABBE Corentin @ 2018-11-07 19:13 UTC (permalink / raw)
To: Eric Biggers; +Cc: davem, herbert, linux-crypto, linux-kernel
On Mon, Nov 05, 2018 at 05:44:19PM -0800, Eric Biggers wrote:
> Hi Corentin,
>
> On Mon, Nov 05, 2018 at 12:51:12PM +0000, Corentin Labbe wrote:
> > It is cleaner to have each stat in their own structures.
> >
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> > crypto/crypto_user_stat.c | 20 +++----
> > include/uapi/linux/cryptouser.h | 101 ++++++++++++++++++++------------
> > 2 files changed, 73 insertions(+), 48 deletions(-)
> >
> > diff --git a/crypto/crypto_user_stat.c b/crypto/crypto_user_stat.c
> > index 352569f378a0..3c14be2f7a1b 100644
> > --- a/crypto/crypto_user_stat.c
> > +++ b/crypto/crypto_user_stat.c
> > @@ -33,7 +33,7 @@ struct crypto_dump_info {
> >
> > static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat raead;
> > + struct crypto_stat_aead raead;
> > u64 v64;
> >
> > memset(&raead, 0, sizeof(raead));
> > @@ -56,7 +56,7 @@ static int crypto_report_aead(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rcipher;
> > + struct crypto_stat_cipher rcipher;
> > u64 v64;
> >
> > memset(&rcipher, 0, sizeof(rcipher));
> > @@ -79,7 +79,7 @@ static int crypto_report_cipher(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rcomp;
> > + struct crypto_stat_compress rcomp;
> > u64 v64;
> >
> > memset(&rcomp, 0, sizeof(rcomp));
> > @@ -101,7 +101,7 @@ static int crypto_report_comp(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat racomp;
> > + struct crypto_stat_compress racomp;
> > u64 v64;
> >
> > memset(&racomp, 0, sizeof(racomp));
> > @@ -123,7 +123,7 @@ static int crypto_report_acomp(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rakcipher;
> > + struct crypto_stat_akcipher rakcipher;
> > u64 v64;
> >
> > memset(&rakcipher, 0, sizeof(rakcipher));
> > @@ -150,7 +150,7 @@ static int crypto_report_akcipher(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rkpp;
> > + struct crypto_stat_kpp rkpp;
> > u64 v;
> >
> > memset(&rkpp, 0, sizeof(rkpp));
> > @@ -171,7 +171,7 @@ static int crypto_report_kpp(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rhash;
> > + struct crypto_stat_hash rhash;
> > u64 v64;
> >
> > memset(&rhash, 0, sizeof(rhash));
> > @@ -190,7 +190,7 @@ static int crypto_report_ahash(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rhash;
> > + struct crypto_stat_hash rhash;
> > u64 v64;
> >
> > memset(&rhash, 0, sizeof(rhash));
> > @@ -209,7 +209,7 @@ static int crypto_report_shash(struct sk_buff *skb, struct crypto_alg *alg)
> >
> > static int crypto_report_rng(struct sk_buff *skb, struct crypto_alg *alg)
> > {
> > - struct crypto_stat rrng;
> > + struct crypto_stat_rng rrng;
> > u64 v64;
> >
> > memset(&rrng, 0, sizeof(rrng));
> > @@ -248,7 +248,7 @@ static int crypto_reportstat_one(struct crypto_alg *alg,
> > if (nla_put_u32(skb, CRYPTOCFGA_PRIORITY_VAL, alg->cra_priority))
> > goto nla_put_failure;
> > if (alg->cra_flags & CRYPTO_ALG_LARVAL) {
> > - struct crypto_stat rl;
> > + struct crypto_stat_larval rl;
> >
> > memset(&rl, 0, sizeof(rl));
> > strscpy(rl.type, "larval", sizeof(rl.type));
> > diff --git a/include/uapi/linux/cryptouser.h b/include/uapi/linux/cryptouser.h
> > index 9f8187077ce4..790b5c6511e5 100644
> > --- a/include/uapi/linux/cryptouser.h
> > +++ b/include/uapi/linux/cryptouser.h
> > @@ -76,47 +76,72 @@ struct crypto_user_alg {
> > __u32 cru_flags;
> > };
> >
> > -struct crypto_stat {
> > - char type[CRYPTO_MAX_NAME];
> > - union {
> > - __u64 stat_encrypt_cnt;
> > - __u64 stat_compress_cnt;
> > - __u64 stat_generate_cnt;
> > - __u64 stat_hash_cnt;
> > - __u64 stat_setsecret_cnt;
> > - };
> > - union {
> > - __u64 stat_encrypt_tlen;
> > - __u64 stat_compress_tlen;
> > - __u64 stat_generate_tlen;
> > - __u64 stat_hash_tlen;
> > - };
> > - union {
> > - __u64 stat_akcipher_err_cnt;
> > - __u64 stat_cipher_err_cnt;
> > - __u64 stat_compress_err_cnt;
> > - __u64 stat_aead_err_cnt;
> > - __u64 stat_hash_err_cnt;
> > - __u64 stat_rng_err_cnt;
> > - __u64 stat_kpp_err_cnt;
> > - };
> > - union {
> > - __u64 stat_decrypt_cnt;
> > - __u64 stat_decompress_cnt;
> > - __u64 stat_seed_cnt;
> > - __u64 stat_generate_public_key_cnt;
> > - };
> > - union {
> > - __u64 stat_decrypt_tlen;
> > - __u64 stat_decompress_tlen;
> > - };
> > - union {
> > - __u64 stat_verify_cnt;
> > - __u64 stat_compute_shared_secret_cnt;
> > - };
> > +struct crypto_stat_cipher {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_encrypt_cnt;
> > + __u64 stat_encrypt_tlen;
> > + __u64 stat_decrypt_cnt;
> > + __u64 stat_decrypt_tlen;
> > + __u64 stat_cipher_err_cnt;
> > +};
> > +
> > +struct crypto_stat_aead {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_encrypt_cnt;
> > + __u64 stat_encrypt_tlen;
> > + __u64 stat_decrypt_cnt;
> > + __u64 stat_decrypt_tlen;
> > + __u64 stat_cipher_err_cnt;
> > + __u64 stat_aead_err_cnt;
> > +};
> > +
> > +struct crypto_stat_akcipher {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_encrypt_cnt;
> > + __u64 stat_encrypt_tlen;
> > + __u64 stat_decrypt_cnt;
> > + __u64 stat_decrypt_tlen;
> > + __u64 stat_akcipher_err_cnt;
> > + __u64 stat_verify_cnt;
> > __u64 stat_sign_cnt;
> > };
> >
> > +struct crypto_stat_compress {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_compress_cnt;
> > + __u64 stat_compress_tlen;
> > + __u64 stat_decompress_cnt;
> > + __u64 stat_decompress_tlen;
> > + __u64 stat_compress_err_cnt;
> > +};
> > +
> > +struct crypto_stat_rng {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_generate_cnt;
> > + __u64 stat_generate_tlen;
> > + __u64 stat_rng_err_cnt;
> > + __u64 stat_seed_cnt;
> > +};
> > +
> > +struct crypto_stat_hash {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_hash_cnt;
> > + __u64 stat_hash_tlen;
> > + __u64 stat_hash_err_cnt;
> > +};
> > +
> > +struct crypto_stat_kpp {
> > + char type[CRYPTO_MAX_NAME];
> > + __u64 stat_setsecret_cnt;
> > + __u64 stat_kpp_err_cnt;
> > + __u64 stat_generate_public_key_cnt;
> > + __u64 stat_compute_shared_secret_cnt;
> > +};
> > +
> > +struct crypto_stat_larval {
> > + char type[CRYPTO_MAX_NAME];
> > +};
> > +
> > struct crypto_report_larval {
> > char type[CRYPTO_MAX_NAME];
> > };
> > --
> > 2.18.1
> >
>
> Is there any particular reason this patch only changes the UAPI structures,
> not the internal structures in 'struct crypto_alg'?
>
While changing it for user space will be clearly better, I didnt see any benefit for the internal ones.
But perhaps it is cleaner to have the sames.
I will do it.
Regards
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request
2018-11-06 1:49 ` Eric Biggers
@ 2018-11-07 19:34 ` LABBE Corentin
0 siblings, 0 replies; 14+ messages in thread
From: LABBE Corentin @ 2018-11-07 19:34 UTC (permalink / raw)
To: Eric Biggers; +Cc: davem, herbert, linux-crypto, linux-kernel
On Mon, Nov 05, 2018 at 05:49:06PM -0800, Eric Biggers wrote:
> Hi Corentin,
>
> On Mon, Nov 05, 2018 at 12:51:14PM +0000, Corentin Labbe wrote:
> > All crypto_stats functions use the struct xxx_request for feeding stats,
> > but in some case this structure could already be freed.
> >
> > For fixing this, the needed parameters (len and alg) will be stored
> > before the request being executed.
> > Fixes: cac5818c25d0 ("crypto: user - Implement a generic crypto statistics")
> > Reported-by: syzbot <syzbot+6939a606a5305e9e9799@syzkaller.appspotmail.com>
> >
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> > crypto/ahash.c | 14 ++++++++--
> > include/crypto/acompress.h | 30 ++++++++++----------
> > include/crypto/aead.h | 30 ++++++++++----------
> > include/crypto/akcipher.h | 56 ++++++++++++++++++--------------------
> > include/crypto/hash.h | 25 +++++++++--------
> > include/crypto/kpp.h | 22 +++++++--------
> > include/crypto/skcipher.h | 16 +++++++----
> > include/linux/crypto.h | 34 +++++++++++------------
> > 8 files changed, 118 insertions(+), 109 deletions(-)
> >
> > diff --git a/crypto/ahash.c b/crypto/ahash.c
> > index 3a348fbcf8f9..3f4c44c1e80f 100644
> > --- a/crypto/ahash.c
> > +++ b/crypto/ahash.c
> > @@ -364,20 +364,26 @@ static int crypto_ahash_op(struct ahash_request *req,
> >
> > int crypto_ahash_final(struct ahash_request *req)
> > {
> > + struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> > + struct crypto_alg *alg = tfm->base.__crt_alg;
> > + unsigned int nbytes = req->nbytes;
> > int ret;
> >
> > ret = crypto_ahash_op(req, crypto_ahash_reqtfm(req)->final);
> > - crypto_stat_ahash_final(req, ret);
> > + crypto_stat_ahash_final(nbytes, ret, alg);
> > return ret;
> > }
> > EXPORT_SYMBOL_GPL(crypto_ahash_final);
>
> I'm not confident this is enough. If the request is being processed
> asynchronously, the completion callback could be used to signal another thread
> to go ahead and free the resources, including not only the request but also the
> 'tfm', which could even result in the last reference to the 'alg' being dropped
> and therefore the 'alg' being freed. If I'm wrong, then please explain why :-)
>
> Note: I'd also prefer a solution that is more obviously zero-overhead in the
> !CONFIG_CRYPTO_STATS case.
>
> - Eric
>
I think the best solution is to grab a crypto_alg refcnt before the operation and release it after.
So for example this will lead to:
--- a/include/crypto/skcipher.h
+++ b/include/crypto/skcipher.h
@@ -486,7 +486,7 @@ static inline struct crypto_sync_skcipher *crypto_sync_skcipher_reqtfm(
return container_of(tfm, struct crypto_sync_skcipher, base);
}
-static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
+static inline void crypto_stat_skcipher_encrypt(unsigned int cryptlen,
int ret, struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
@@ -494,12 +494,13 @@ static inline void crypto_stat_skcipher_encrypt(struct skcipher_request *req,
atomic64_inc(&alg->cipher_err_cnt);
} else {
atomic64_inc(&alg->encrypt_cnt);
- atomic64_add(req->cryptlen, &alg->encrypt_tlen);
+ atomic64_add(cryptlen, &alg->encrypt_tlen);
}
+ crypto_alg_put(alg);
#endif
}
-static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
+static inline void crypto_stat_skcipher_decrypt(unsigned int cryptlen,
int ret, struct crypto_alg *alg)
{
#ifdef CONFIG_CRYPTO_STATS
@@ -507,8 +508,9 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
atomic64_inc(&alg->cipher_err_cnt);
} else {
atomic64_inc(&alg->decrypt_cnt);
- atomic64_add(req->cryptlen, &alg->decrypt_tlen);
+ atomic64_add(cryptlen, &alg->decrypt_tlen);
}
+ crypto_alg_put(alg);
#endif
}
@@ -526,13 +528,18 @@ static inline void crypto_stat_skcipher_decrypt(struct skcipher_request *req,
static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int cryptlen = req->cryptlen;
int ret;
+#ifdef CONFIG_CRYPTO_STATS
+ crypto_alg_get(alg);
+#endif
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = tfm->encrypt(req);
- crypto_stat_skcipher_encrypt(req, ret, tfm->base.__crt_alg);
+ crypto_stat_skcipher_encrypt(cryptlen, ret, alg);
return ret;
}
@@ -550,13 +557,18 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
static inline int crypto_skcipher_decrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ struct crypto_alg *alg = tfm->base.__crt_alg;
+ unsigned int cryptlen = req->cryptlen;
int ret;
+#ifdef CONFIG_CRYPTO_STATS
+ crypto_alg_get(alg);
+#endif
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
ret = -ENOKEY;
else
ret = tfm->decrypt(req);
- crypto_stat_skcipher_decrypt(req, ret, tfm->base.__crt_alg);
+ crypto_stat_skcipher_decrypt(cryptlen, ret, alg);
return ret;
}
This should not have any overhead with !CONFIG_CRYPTO_STATS
The only drawback is that crypto_alg_get/crypto_alg_put need to be moved out of crypto/internal.h to include/linux/crypto.h
Herbert what do you think about that ?
Regards
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-11-08 5:06 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05 12:51 [PATCH 0/5] crypto: crypto_user_stat: misc enhancement Corentin Labbe
2018-11-05 12:51 ` [PATCH 1/5] crypto: crypto_user_stat: made crypto_user_stat optional Corentin Labbe
2018-11-06 1:41 ` Eric Biggers
2018-11-07 18:55 ` LABBE Corentin
2018-11-05 12:51 ` [PATCH 2/5] crypto: crypto_user_stat: convert all stats from u32 to u64 Corentin Labbe
2018-11-06 1:42 ` Eric Biggers
2018-11-07 18:58 ` LABBE Corentin
2018-11-05 12:51 ` [PATCH 3/5] crypto: crypto_user_stat: split user space crypto stat structures Corentin Labbe
2018-11-06 1:44 ` Eric Biggers
2018-11-07 19:13 ` LABBE Corentin
2018-11-05 12:51 ` [PATCH 4/5] crypto: tool: getstat: convert user space example to the new crypto_user_stat uapi Corentin Labbe
2018-11-05 12:51 ` [PATCH 5/5] crypto: crypto_user_stat: fix use_after_free of struct xxx_request Corentin Labbe
2018-11-06 1:49 ` Eric Biggers
2018-11-07 19:34 ` LABBE Corentin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.