Re: [PATCH] crypto: shash - stop comparing function pointers to avoid breaking CFI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 04, 2021 at 09:00:09PM +0200, Ard Biesheuvel wrote:
> 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, which makes the problem go away.
> 
> Cc: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> ---
>  crypto/shash.c                 | 11 ++++++++---
>  include/crypto/internal/hash.h |  8 +-------
>  2 files changed, 9 insertions(+), 10 deletions(-)

I think this deserves a comment in the code.

> +bool crypto_shash_alg_has_setkey(struct shash_alg *alg)
> +{
> +       return alg->setkey != shash_no_setkey;
> +}
> +EXPORT_SYMBOL_GPL(crypto_shash_alg_has_setkey);

This would be a good spot for the comment so someone doesn't try
to make it inline again later.

Thanks,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux