From 3906f640224dbe7714b52b66d7d68c0812808e19 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Fri, 5 Jun 2020 16:59:18 +1000 Subject: crc-t10dif: Fix potential crypto notify dead-lock The crypto notify call occurs with a read mutex held so you must not do any substantial work directly. In particular, you cannot call crypto_alloc_* as they may trigger further notifications which may dead-lock in the presence of another writer. This patch fixes this by postponing the work into a work queue and taking the same lock in the module init function. While we're at it this patch also ensures that all RCU accesses are marked appropriately (tested with sparse). Finally this also reveals a race condition in module param show function as it may be called prior to the module init function. It's fixed by testing whether crct10dif_tfm is NULL (this is true iff the init function has not completed assuming fallback is false). Fixes: 11dcb1037f40 ("crc-t10dif: Allow current transform to be...") Fixes: b76377543b73 ("crc-t10dif: Pick better transform if one...") Signed-off-by: Herbert Xu Reviewed-by: Martin K. Petersen Reviewed-by: Eric Biggers Signed-off-by: Herbert Xu --- lib/crc-t10dif.c | 54 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c index 8cc01a603416..c9acf1c12cfc 100644 --- a/lib/crc-t10dif.c +++ b/lib/crc-t10dif.c @@ -19,39 +19,46 @@ static struct crypto_shash __rcu *crct10dif_tfm; static struct static_key crct10dif_fallback __read_mostly; static DEFINE_MUTEX(crc_t10dif_mutex); +static struct work_struct crct10dif_rehash_work; -static int crc_t10dif_rehash(struct notifier_block *self, unsigned long val, void *data) +static int crc_t10dif_notify(struct notifier_block *self, unsigned long val, void *data) { struct crypto_alg *alg = data; - struct crypto_shash *new, *old; if (val != CRYPTO_MSG_ALG_LOADED || static_key_false(&crct10dif_fallback) || strncmp(alg->cra_name, CRC_T10DIF_STRING, strlen(CRC_T10DIF_STRING))) return 0; + schedule_work(&crct10dif_rehash_work); + return 0; +} + +static void crc_t10dif_rehash(struct work_struct *work) +{ + struct crypto_shash *new, *old; + mutex_lock(&crc_t10dif_mutex); old = rcu_dereference_protected(crct10dif_tfm, lockdep_is_held(&crc_t10dif_mutex)); if (!old) { mutex_unlock(&crc_t10dif_mutex); - return 0; + return; } new = crypto_alloc_shash("crct10dif", 0, 0); if (IS_ERR(new)) { mutex_unlock(&crc_t10dif_mutex); - return 0; + return; } rcu_assign_pointer(crct10dif_tfm, new); mutex_unlock(&crc_t10dif_mutex); synchronize_rcu(); crypto_free_shash(old); - return 0; } static struct notifier_block crc_t10dif_nb = { - .notifier_call = crc_t10dif_rehash, + .notifier_call = crc_t10dif_notify, }; __u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len) @@ -86,19 +93,26 @@ EXPORT_SYMBOL(crc_t10dif); static int __init crc_t10dif_mod_init(void) { + struct crypto_shash *tfm; + + INIT_WORK(&crct10dif_rehash_work, crc_t10dif_rehash); crypto_register_notifier(&crc_t10dif_nb); - crct10dif_tfm = crypto_alloc_shash("crct10dif", 0, 0); - if (IS_ERR(crct10dif_tfm)) { + mutex_lock(&crc_t10dif_mutex); + tfm = crypto_alloc_shash("crct10dif", 0, 0); + if (IS_ERR(tfm)) { static_key_slow_inc(&crct10dif_fallback); - crct10dif_tfm = NULL; + tfm = NULL; } + RCU_INIT_POINTER(crct10dif_tfm, tfm); + mutex_unlock(&crc_t10dif_mutex); return 0; } static void __exit crc_t10dif_mod_fini(void) { crypto_unregister_notifier(&crc_t10dif_nb); - crypto_free_shash(crct10dif_tfm); + cancel_work_sync(&crct10dif_rehash_work); + crypto_free_shash(rcu_dereference_protected(crct10dif_tfm, 1)); } module_init(crc_t10dif_mod_init); @@ -106,11 +120,27 @@ module_exit(crc_t10dif_mod_fini); static int crc_t10dif_transform_show(char *buffer, const struct kernel_param *kp) { + struct crypto_shash *tfm; + const char *name; + int len; + if (static_key_false(&crct10dif_fallback)) return sprintf(buffer, "fallback\n"); - return sprintf(buffer, "%s\n", - crypto_tfm_alg_driver_name(crypto_shash_tfm(crct10dif_tfm))); + rcu_read_lock(); + tfm = rcu_dereference(crct10dif_tfm); + if (!tfm) { + len = sprintf(buffer, "init\n"); + goto unlock; + } + + name = crypto_tfm_alg_driver_name(crypto_shash_tfm(tfm)); + len = sprintf(buffer, "%s\n", name); + +unlock: + rcu_read_unlock(); + + return len; } module_param_call(transform, NULL, crc_t10dif_transform_show, NULL, 0644); -- cgit v1.2.3-70-g09d2 From be924e0aaa315a179c25221685e3f1a35a70156e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 9 Jun 2020 23:39:42 -0700 Subject: crc-t10dif: use fallback in initial state Currently the crc-t10dif module starts out with the fallback disabled and crct10dif_tfm == NULL. crc_t10dif_mod_init() tries to allocate crct10dif_tfm, and if it fails it enables the fallback. This is backwards because it means that any call to crc_t10dif() prior to module_init (which could theoretically happen from built-in code) will crash rather than use the fallback as expected. Also, it means that if the initial tfm allocation fails, then the fallback stays permanently enabled even if a crct10dif implementation is loaded later. Change it to use the more logical solution of starting with the fallback enabled, and disabling the fallback when a tfm gets allocated for the first time. This change also ends up simplifying the code. Also take the opportunity to convert the code to use the new static_key API, which is much less confusing than the old and deprecated one. Cc: Martin K. Petersen Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- lib/crc-t10dif.c | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) (limited to 'lib') diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c index c9acf1c12cfc..af3613367ef9 100644 --- a/lib/crc-t10dif.c +++ b/lib/crc-t10dif.c @@ -17,7 +17,7 @@ #include static struct crypto_shash __rcu *crct10dif_tfm; -static struct static_key crct10dif_fallback __read_mostly; +static DEFINE_STATIC_KEY_TRUE(crct10dif_fallback); static DEFINE_MUTEX(crc_t10dif_mutex); static struct work_struct crct10dif_rehash_work; @@ -26,7 +26,6 @@ static int crc_t10dif_notify(struct notifier_block *self, unsigned long val, voi struct crypto_alg *alg = data; if (val != CRYPTO_MSG_ALG_LOADED || - static_key_false(&crct10dif_fallback) || strncmp(alg->cra_name, CRC_T10DIF_STRING, strlen(CRC_T10DIF_STRING))) return 0; @@ -41,10 +40,6 @@ static void crc_t10dif_rehash(struct work_struct *work) mutex_lock(&crc_t10dif_mutex); old = rcu_dereference_protected(crct10dif_tfm, lockdep_is_held(&crc_t10dif_mutex)); - if (!old) { - mutex_unlock(&crc_t10dif_mutex); - return; - } new = crypto_alloc_shash("crct10dif", 0, 0); if (IS_ERR(new)) { mutex_unlock(&crc_t10dif_mutex); @@ -53,8 +48,12 @@ static void crc_t10dif_rehash(struct work_struct *work) rcu_assign_pointer(crct10dif_tfm, new); mutex_unlock(&crc_t10dif_mutex); - synchronize_rcu(); - crypto_free_shash(old); + if (old) { + synchronize_rcu(); + crypto_free_shash(old); + } else { + static_branch_disable(&crct10dif_fallback); + } } static struct notifier_block crc_t10dif_nb = { @@ -69,7 +68,7 @@ __u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len) } desc; int err; - if (static_key_false(&crct10dif_fallback)) + if (static_branch_unlikely(&crct10dif_fallback)) return crc_t10dif_generic(crc, buffer, len); rcu_read_lock(); @@ -93,18 +92,9 @@ EXPORT_SYMBOL(crc_t10dif); static int __init crc_t10dif_mod_init(void) { - struct crypto_shash *tfm; - INIT_WORK(&crct10dif_rehash_work, crc_t10dif_rehash); crypto_register_notifier(&crc_t10dif_nb); - mutex_lock(&crc_t10dif_mutex); - tfm = crypto_alloc_shash("crct10dif", 0, 0); - if (IS_ERR(tfm)) { - static_key_slow_inc(&crct10dif_fallback); - tfm = NULL; - } - RCU_INIT_POINTER(crct10dif_tfm, tfm); - mutex_unlock(&crc_t10dif_mutex); + crc_t10dif_rehash(&crct10dif_rehash_work); return 0; } @@ -124,20 +114,13 @@ static int crc_t10dif_transform_show(char *buffer, const struct kernel_param *kp const char *name; int len; - if (static_key_false(&crct10dif_fallback)) + if (static_branch_unlikely(&crct10dif_fallback)) return sprintf(buffer, "fallback\n"); rcu_read_lock(); tfm = rcu_dereference(crct10dif_tfm); - if (!tfm) { - len = sprintf(buffer, "init\n"); - goto unlock; - } - name = crypto_tfm_alg_driver_name(crypto_shash_tfm(tfm)); len = sprintf(buffer, "%s\n", name); - -unlock: rcu_read_unlock(); return len; -- cgit v1.2.3-70-g09d2 From 29195232fa2f722449c1c6850277b4ca9fa5f781 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 9 Jun 2020 23:39:43 -0700 Subject: crc-t10dif: clean up some more things - Correctly compare the algorithm name in crc_t10dif_notify(). - Use proper NOTIFY_* status codes instead of 0. - Consistently use CRC_T10DIF_STRING instead of "crct10dif" directly. - Use a proper type for the shash_desc context. - Use crypto_shash_driver_name() instead of open-coding it. - Make crc_t10dif_transform_show() use snprintf() rather than sprintf(). This isn't actually necessary since the buffer has size PAGE_SIZE and CRYPTO_MAX_ALG_NAME < PAGE_SIZE, but it's good practice. - Give the "transform" sysfs file mode 0444 rather than 0644, since it doesn't implement a setter method. - Adjust the module description to not be the same as crct10dif-generic. Cc: Martin K. Petersen Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- lib/crc-t10dif.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c index af3613367ef9..1ed2ed487097 100644 --- a/lib/crc-t10dif.c +++ b/lib/crc-t10dif.c @@ -26,11 +26,11 @@ static int crc_t10dif_notify(struct notifier_block *self, unsigned long val, voi struct crypto_alg *alg = data; if (val != CRYPTO_MSG_ALG_LOADED || - strncmp(alg->cra_name, CRC_T10DIF_STRING, strlen(CRC_T10DIF_STRING))) - return 0; + strcmp(alg->cra_name, CRC_T10DIF_STRING)) + return NOTIFY_DONE; schedule_work(&crct10dif_rehash_work); - return 0; + return NOTIFY_OK; } static void crc_t10dif_rehash(struct work_struct *work) @@ -40,7 +40,7 @@ static void crc_t10dif_rehash(struct work_struct *work) mutex_lock(&crc_t10dif_mutex); old = rcu_dereference_protected(crct10dif_tfm, lockdep_is_held(&crc_t10dif_mutex)); - new = crypto_alloc_shash("crct10dif", 0, 0); + new = crypto_alloc_shash(CRC_T10DIF_STRING, 0, 0); if (IS_ERR(new)) { mutex_unlock(&crc_t10dif_mutex); return; @@ -64,7 +64,7 @@ __u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len) { struct { struct shash_desc shash; - char ctx[2]; + __u16 crc; } desc; int err; @@ -73,14 +73,13 @@ __u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len) rcu_read_lock(); desc.shash.tfm = rcu_dereference(crct10dif_tfm); - *(__u16 *)desc.ctx = crc; - + desc.crc = crc; err = crypto_shash_update(&desc.shash, buffer, len); rcu_read_unlock(); BUG_ON(err); - return *(__u16 *)desc.ctx; + return desc.crc; } EXPORT_SYMBOL(crc_t10dif_update); @@ -111,7 +110,6 @@ module_exit(crc_t10dif_mod_fini); static int crc_t10dif_transform_show(char *buffer, const struct kernel_param *kp) { struct crypto_shash *tfm; - const char *name; int len; if (static_branch_unlikely(&crct10dif_fallback)) @@ -119,15 +117,15 @@ static int crc_t10dif_transform_show(char *buffer, const struct kernel_param *kp rcu_read_lock(); tfm = rcu_dereference(crct10dif_tfm); - name = crypto_tfm_alg_driver_name(crypto_shash_tfm(tfm)); - len = sprintf(buffer, "%s\n", name); + len = snprintf(buffer, PAGE_SIZE, "%s\n", + crypto_shash_driver_name(tfm)); rcu_read_unlock(); return len; } -module_param_call(transform, NULL, crc_t10dif_transform_show, NULL, 0644); +module_param_call(transform, NULL, crc_t10dif_transform_show, NULL, 0444); -MODULE_DESCRIPTION("T10 DIF CRC calculation"); +MODULE_DESCRIPTION("T10 DIF CRC calculation (library API)"); MODULE_LICENSE("GPL"); MODULE_SOFTDEP("pre: crct10dif"); -- cgit v1.2.3-70-g09d2 From 06cc2afbbdf9a9e8df3e2f8db724997dd6e1b4ac Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Wed, 8 Jul 2020 12:41:13 +1000 Subject: crypto: lib/chacha20poly1305 - Add missing function declaration This patch adds a declaration for chacha20poly1305_selftest to silence a sparse warning. Signed-off-by: Herbert Xu --- include/crypto/chacha20poly1305.h | 2 ++ lib/crypto/chacha20poly1305.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/include/crypto/chacha20poly1305.h b/include/crypto/chacha20poly1305.h index 234ee28078ef..d2ac3ff7dc1e 100644 --- a/include/crypto/chacha20poly1305.h +++ b/include/crypto/chacha20poly1305.h @@ -45,4 +45,6 @@ bool chacha20poly1305_decrypt_sg_inplace(struct scatterlist *src, size_t src_len const u64 nonce, const u8 key[CHACHA20POLY1305_KEY_SIZE]); +bool chacha20poly1305_selftest(void); + #endif /* __CHACHA20POLY1305_H */ diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c index ad0699ce702f..431e04280332 100644 --- a/lib/crypto/chacha20poly1305.c +++ b/lib/crypto/chacha20poly1305.c @@ -21,8 +21,6 @@ #define CHACHA_KEY_WORDS (CHACHA_KEY_SIZE / sizeof(u32)) -bool __init chacha20poly1305_selftest(void); - static void chacha_load_key(u32 *k, const u8 *in) { k[0] = get_unaligned_le32(in); -- cgit v1.2.3-70-g09d2 From 9ea9c58b40a441a0babef8c615acedcfb3733919 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 8 Jul 2020 09:39:40 -0700 Subject: crypto: lib/sha256 - add sha256() function Add a function sha256() which computes a SHA-256 digest in one step, combining sha256_init() + sha256_update() + sha256_final(). This is similar to how we also have blake2s(). Reviewed-by: Ard Biesheuvel Tested-by: Hans de Goede Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- include/crypto/sha.h | 1 + lib/crypto/sha256.c | 10 ++++++++++ 2 files changed, 11 insertions(+) (limited to 'lib') diff --git a/include/crypto/sha.h b/include/crypto/sha.h index 10753ff71d46..4ff3da816630 100644 --- a/include/crypto/sha.h +++ b/include/crypto/sha.h @@ -147,6 +147,7 @@ static inline void sha256_init(struct sha256_state *sctx) } void sha256_update(struct sha256_state *sctx, const u8 *data, unsigned int len); void sha256_final(struct sha256_state *sctx, u8 *out); +void sha256(const u8 *data, unsigned int len, u8 *out); static inline void sha224_init(struct sha256_state *sctx) { diff --git a/lib/crypto/sha256.c b/lib/crypto/sha256.c index 2e621697c5c3..2321f6cb322f 100644 --- a/lib/crypto/sha256.c +++ b/lib/crypto/sha256.c @@ -280,4 +280,14 @@ void sha224_final(struct sha256_state *sctx, u8 *out) } EXPORT_SYMBOL(sha224_final); +void sha256(const u8 *data, unsigned int len, u8 *out) +{ + struct sha256_state sctx; + + sha256_init(&sctx); + sha256_update(&sctx, data, len); + sha256_final(&sctx, out); +} +EXPORT_SYMBOL(sha256); + MODULE_LICENSE("GPL"); -- cgit v1.2.3-70-g09d2 From 4278e9d99e38938a7611b927fa4d73e6c86cb4fc Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Cerri Date: Mon, 20 Jul 2020 19:08:09 +0200 Subject: lib/mpi: Add mpi_sub_ui() Add mpi_sub_ui() based on Gnu MP mpz_sub_ui() function from file mpz/aors_ui.h[1] from change id 510b83519d1c adapting the code to the kernel's data structures, helper functions and coding style and also removing the defines used to produce mpz_sub_ui() and mpz_add_ui() from the same code. [1] https://gmplib.org/repo/gmp-6.2/file/510b83519d1c/mpz/aors.h Signed-off-by: Marcelo Henrique Cerri Signed-off-by: Stephan Mueller Signed-off-by: Herbert Xu --- include/linux/mpi.h | 3 ++ lib/mpi/Makefile | 1 + lib/mpi/mpi-sub-ui.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 lib/mpi/mpi-sub-ui.c (limited to 'lib') diff --git a/include/linux/mpi.h b/include/linux/mpi.h index 7bd6d8af0004..5d906dfbf3ed 100644 --- a/include/linux/mpi.h +++ b/include/linux/mpi.h @@ -63,6 +63,9 @@ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod); int mpi_cmp_ui(MPI u, ulong v); int mpi_cmp(MPI u, MPI v); +/*-- mpi-sub-ui.c --*/ +int mpi_sub_ui(MPI w, MPI u, unsigned long vval); + /*-- mpi-bit.c --*/ void mpi_normalize(MPI a); unsigned mpi_get_nbits(MPI a); diff --git a/lib/mpi/Makefile b/lib/mpi/Makefile index d5874a7f5ff9..43b8fce14079 100644 --- a/lib/mpi/Makefile +++ b/lib/mpi/Makefile @@ -16,6 +16,7 @@ mpi-y = \ mpicoder.o \ mpi-bit.o \ mpi-cmp.o \ + mpi-sub-ui.o \ mpih-cmp.o \ mpih-div.o \ mpih-mul.o \ diff --git a/lib/mpi/mpi-sub-ui.c b/lib/mpi/mpi-sub-ui.c new file mode 100644 index 000000000000..b41b082b5f3e --- /dev/null +++ b/lib/mpi/mpi-sub-ui.c @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* mpi-sub-ui.c - Subtract an unsigned integer from an MPI. + * + * Copyright 1991, 1993, 1994, 1996, 1999-2002, 2004, 2012, 2013, 2015 + * Free Software Foundation, Inc. + * + * This file was based on the GNU MP Library source file: + * https://gmplib.org/repo/gmp-6.2/file/510b83519d1c/mpz/aors_ui.h + * + * The GNU MP Library is free software; you can redistribute it and/or modify + * it under the terms of either: + * + * * the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 3 of the License, or (at your + * option) any later version. + * + * or + * + * * the GNU General Public License as published by the Free Software + * Foundation; either version 2 of the License, or (at your option) any + * later version. + * + * or both in parallel, as here. + * + * The GNU MP Library is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + * + * You should have received copies of the GNU General Public License and the + * GNU Lesser General Public License along with the GNU MP Library. If not, + * see https://www.gnu.org/licenses/. + */ + +#include "mpi-internal.h" + +int mpi_sub_ui(MPI w, MPI u, unsigned long vval) +{ + if (u->nlimbs == 0) { + if (mpi_resize(w, 1) < 0) + return -ENOMEM; + w->d[0] = vval; + w->nlimbs = (vval != 0); + w->sign = (vval != 0); + return 0; + } + + /* If not space for W (and possible carry), increase space. */ + if (mpi_resize(w, u->nlimbs + 1)) + return -ENOMEM; + + if (u->sign) { + mpi_limb_t cy; + + cy = mpihelp_add_1(w->d, u->d, u->nlimbs, (mpi_limb_t) vval); + w->d[u->nlimbs] = cy; + w->nlimbs = u->nlimbs + cy; + w->sign = 1; + } else { + /* The signs are different. Need exact comparison to determine + * which operand to subtract from which. + */ + if (u->nlimbs == 1 && u->d[0] < vval) { + w->d[0] = vval - u->d[0]; + w->nlimbs = 1; + w->sign = 1; + } else { + mpihelp_sub_1(w->d, u->d, u->nlimbs, (mpi_limb_t) vval); + /* Size can decrease with at most one limb. */ + w->nlimbs = (u->nlimbs - (w->d[u->nlimbs - 1] == 0)); + w->sign = 0; + } + } + + mpi_normalize(w); + return 0; +} +EXPORT_SYMBOL_GPL(mpi_sub_ui); -- cgit v1.2.3-70-g09d2