crypto: shash - avoid comparing pointers to exported functions under CFI
PATCH v3 1/1 crypto: shash - avoid comparing pointers to exported functions under CFI
crypto_shash_alg_has_setkey() is implemented by testing whether the .setkey() member of a struct shash_alg points to the default version, called shash_no_setkey(). As crypto_shash_alg_has_setkey() is a static inline, this requires shash_no_setkey() to be exported to modules.
Unfortunately, when building with CFI, function pointers are routed via CFI stubs which are private to each module (or to the kernel proper) and so this function pointer comparison may fail spuriously.
Let's fix this by turning crypto_shash_alg_has_setkey() into an out of line function.
---
v3: improve comment as per Eric's suggestion
v2: add code comment to explain why the function needs to remain out of line
crypto/shash.c | 18 +++++++++++++++--- include/crypto/internal/hash.h | 8 +------- 2 files changed, 16 insertions(+), 10 deletions(-)
crypto/shash.c +15 -3
@@ -20,12 +20,24 @@static const struct crypto_type crypto_shash_type; -int shash_no_setkey (struct crypto_shash *tfm,const u8 *key, -unsigned int keylen) +static int shash_no_setkey (struct crypto_shash *tfm,const u8 *key, +unsigned int keylen) {return -ENOSYS ; } -EXPORT_SYMBOL_GPL (shash_no_setkey); + +/* +* Check whether an shash algorithm has a setkey function. +* +* For CFI compatibility, this must not be an inline function. This is because +* when CFI is enabled, modules won't get the same address for shash_no_setkey +* (if it were exported, which inlining would require) as the core kernel will. +*/ +bool crypto_shash_alg_has_setkey (struct shash_alg *alg) +{ +return alg->setkey != shash_no_setkey; +} +EXPORT_SYMBOL_GPL (crypto_shash_alg_has_setkey);static int shash_setkey_unaligned (struct crypto_shash *tfm,const u8 *key,unsigned int keylen)
include/crypto/internal/hash.h +1 -7
@@ -75,13 +75,7 @@ void crypto_unregister_ahashes(struct ahash_alg *algs, int count);int ahash_register_instance (struct crypto_template *tmpl,struct ahash_instance *inst); -int shash_no_setkey (struct crypto_shash *tfm,const u8 *key, -unsigned int keylen); - -static inline bool crypto_shash_alg_has_setkey (struct shash_alg *alg) -{ -return alg->setkey != shash_no_setkey; -} +bool crypto_shash_alg_has_setkey (struct shash_alg *alg);static inline bool crypto_shash_alg_needs_key (struct shash_alg *alg) {
Signature · 1 line
2.30.2
Quoted 15 lines · Ard Biesheuvel
crypto_shash_alg_has_setkey()is implemented by testing whether the .setkey() member of a structshash_algpoints to the default version, calledshash_no_setkey(). Ascrypto_shash_alg_has_setkey()is a static inline, this requiresshash_no_setkey()to be exported to modules.Unfortunately, when building with CFI, function pointers are routed via CFI stubs which are private to each module (or to the kernel proper) and so this function pointer comparison may fail spuriously.
Let's fix this by turning
crypto_shash_alg_has_setkey()into an out of line function.Cc Sami Tolvanen <[email protected]>Cc Eric Biggers <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>
Quoted 9 lines
---
v3: improve comment as per Eric's suggestion
v2: add code comment to explain why the function needs to remain out of linecrypto/shash.c | 18 +++++++++++++++--- include/crypto/internal/hash.h | 8 +------- 2 files changed, 16 insertions(+), 10 deletions(-)
Sami Tolvanen
Reviewed-byQuoted 22 lines · Eric Biggers
Quoted 20 lines · Ard Biesheuvel
crypto_shash_alg_has_setkey()is implemented by testing whether the .setkey() member of a structshash_algpoints to the default version, calledshash_no_setkey(). Ascrypto_shash_alg_has_setkey()is a static inline, this requiresshash_no_setkey()to be exported to modules.Unfortunately, when building with CFI, function pointers are routed via CFI stubs which are private to each module (or to the kernel proper) and so this function pointer comparison may fail spuriously.
Let's fix this by turning
crypto_shash_alg_has_setkey()into an out of line function.Cc Sami Tolvanen <[email protected]>Cc Eric Biggers <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>---
v3: improve comment as per Eric's suggestion
v2: add code comment to explain why the function needs to remain out of line
Quoted 6 lines
Quoted 4 lines
crypto/shash.c | 18 +++++++++++++++--- include/crypto/internal/hash.h | 8 +------- 2 files changed, 16 insertions(+), 10 deletions(-)Reviewed-by Eric Biggers <[email protected]>
Looks good to me as well. Thank you for fixing this!
Sami
Herbert Xu
maintainerQuoted 15 lines · Ard Biesheuvel
crypto_shash_alg_has_setkey()is implemented by testing whether the .setkey() member of a structshash_algpoints to the default version, calledshash_no_setkey(). Ascrypto_shash_alg_has_setkey()is a static inline, this requiresshash_no_setkey()to be exported to modules.Unfortunately, when building with CFI, function pointers are routed via CFI stubs which are private to each module (or to the kernel proper) and so this function pointer comparison may fail spuriously.
Let's fix this by turning
crypto_shash_alg_has_setkey()into an out of line function.Cc Sami Tolvanen <[email protected]>Cc Eric Biggers <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>
Quoted 8 lines
---
v3: improve comment as per Eric's suggestion
v2: add code comment to explain why the function needs to remain out of linecrypto/shash.c | 18 +++++++++++++++--- include/crypto/internal/hash.h | 8 +------- 2 files changed, 16 insertions(+), 10 deletions(-)
Patch applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt