Skip to contentSkip to conversation

x86/sgx: Use SHA-256 library API instead of crypto_shash API

PATCH In review Eric Biggers 3 participants 4 messages Apr 28, 2025 – Apr 30, 2025

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 x86/sgx: Use SHA-256 library API instead of crypto_shash API

From: Eric Biggers <[email protected]>

This user of SHA-256 does not support any other algorithm, so the crypto_shash abstraction provides no value. Just use the SHA-256 library API instead, which is much simpler and easier to use.

---

This patch is targeting the x86 tree for 6.16.

 arch/x86/Kconfig                 |  3 +--
 arch/x86/kernel/cpu/sgx/driver.h |  1 -
 arch/x86/kernel/cpu/sgx/ioctl.c  | 30 ++----------------------------
 3 files changed, 3 insertions(+), 31 deletions(-)
arch/x86/Kconfig +1 -2
index faafd99e52292..bf4874ba4fcfb 100644
@@ -1880,12 +1880,11 @@ config X86_INTEL_TSX_MODE_AUTO
 endchoice
 
 config X86_SGX
 	bool "Software Guard eXtensions (SGX)"
 	depends on X86_64 && CPU_SUP_INTEL && X86_X2APIC
-	depends on CRYPTO=y
-	depends on CRYPTO_SHA256=y
+	select CRYPTO_LIB_SHA256
 	select MMU_NOTIFIER
 	select NUMA_KEEP_MEMINFO if NUMA
 	select XARRAY_MULTI
 	help
 	  Intel(R) Software Guard eXtensions (SGX) is a set of CPU instructions
arch/x86/kernel/cpu/sgx/driver.h +0 -1
index 4eddb4d571ef2..30f39f92c98fc 100644
@@ -1,10 +1,9 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef __ARCH_SGX_DRIVER_H__
 #define __ARCH_SGX_DRIVER_H__
 
-#include <crypto/hash.h>
 #include <linux/kref.h>
 #include <linux/mmu_notifier.h>
 #include <linux/radix-tree.h>
 #include <linux/rwsem.h>
 #include <linux/sched.h>
arch/x86/kernel/cpu/sgx/ioctl.c +2 -28
index 776a20172867e..66f1efa16fbb7 100644
@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 /*  Copyright(c) 2016-20 Intel Corporation. */
 
 #include <asm/mman.h>
 #include <asm/sgx.h>
+#include <crypto/sha2.h>
 #include <linux/mman.h>
 #include <linux/delay.h>
 #include <linux/file.h>
 #include <linux/hashtable.h>
 #include <linux/highmem.h>
@@ -461,35 +462,10 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
 		return -EFAULT;
 
 	return ret;
 }
 
-static int __sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus,
-			      void *hash)
-{
-	SHASH_DESC_ON_STACK(shash, tfm);
-
-	shash->tfm = tfm;
-
-	return crypto_shash_digest(shash, modulus, SGX_MODULUS_SIZE, hash);
-}
-
-static int sgx_get_key_hash(const void *modulus, void *hash)
-{
-	struct crypto_shash *tfm;
-	int ret;
-
-	tfm = crypto_alloc_shash("sha256", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(tfm))
-		return PTR_ERR(tfm);
-
-	ret = __sgx_get_key_hash(tfm, modulus, hash);
-
-	crypto_free_shash(tfm);
-	return ret;
-}
-
 static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
 			 void *token)
 {
 	u64 mrsigner[4];
 	int i, j;
@@ -521,13 +497,11 @@ static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
 
 	if (sigstruct->body.xfrm & sigstruct->body.xfrm_mask &
 	    sgx_xfrm_reserved_mask)
 		return -EINVAL;
 
-	ret = sgx_get_key_hash(sigstruct->modulus, mrsigner);
-	if (ret)
-		return ret;
+	sha256(sigstruct->modulus, SGX_MODULUS_SIZE, (u8 *)mrsigner);
 
 	mutex_lock(&encl->lock);
 
 	/*
 	 * ENCLS[EINIT] is interruptible because it has such a high latency,

base-commit: 33035b665157558254b3c21c3f049fd728e72368

Signature · 1 line
2.49.0
#1 lore raw
Quoted 4 lines · Eric Biggers
arch/x86/kernel/cpu/sgx/ioctl.c +0 -4
-static int sgx_get_key_hash(const void *modulus, void *hash)
-{
-	struct crypto_shash *tfm;
-	int ret;

arch/x86/kernel/cpu/sgx/ioctl.c static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg) · line 489

Quoted 10 lines
arch/x86/kernel/cpu/sgx/ioctl.c +0 -10
-
-	tfm = crypto_alloc_shash("sha256", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(tfm))
-		return PTR_ERR(tfm);
-
-	ret = __sgx_get_key_hash(tfm, modulus, hash);
-
-	crypto_free_shash(tfm);
-	return ret;
-}

Let's just say, theoretically, that there was some future hardware that also supported SHA384. There doesn't seem to be a SHA-384 library API.

Would you leave the crypto_shash() in place if that were to be happening? Theoretically of course.

#2 lore raw
Quoted 19 lines · Dave Hansen
Quoted 4 lines · Eric Biggers
arch/x86/kernel/cpu/sgx/ioctl.c +0 -4
-static int sgx_get_key_hash(const void *modulus, void *hash)
-{
-	struct crypto_shash *tfm;
-	int ret;

arch/x86/kernel/cpu/sgx/ioctl.c static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg) · line 489

Quoted 10 lines
arch/x86/kernel/cpu/sgx/ioctl.c +0 -10
-
-	tfm = crypto_alloc_shash("sha256", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(tfm))
-		return PTR_ERR(tfm);
-
-	ret = __sgx_get_key_hash(tfm, modulus, hash);
-
-	crypto_free_shash(tfm);
-	return ret;
-}

Let's just say, theoretically, that there was some future hardware that also supported SHA384. There doesn't seem to be a SHA-384 library API.

Quoted 2 lines

Would you leave the crypto_shash() in place if that were to be happening? Theoretically of course.

Currently SHA-384 is only available via crypto_shash, but I'm planning to add a library API for that too. (Well, I actually want SHA-512, but it's straightforward to support SHA-384 alongside that.) It's up to you if you'd then want to use crypto_shash vs. the library functions for both, but I'd lean towards just using the library functions.

- Eric

maintainer
#3 lore raw
In reply to Dave Hansen
Quoted 19 lines · Dave Hansen
Quoted 4 lines · Eric Biggers
arch/x86/kernel/cpu/sgx/ioctl.c +0 -4
-static int sgx_get_key_hash(const void *modulus, void *hash)
-{
-	struct crypto_shash *tfm;
-	int ret;

arch/x86/kernel/cpu/sgx/ioctl.c static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg) · line 489

Quoted 10 lines
arch/x86/kernel/cpu/sgx/ioctl.c +0 -10
-
-	tfm = crypto_alloc_shash("sha256", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(tfm))
-		return PTR_ERR(tfm);
-
-	ret = __sgx_get_key_hash(tfm, modulus, hash);
-
-	crypto_free_shash(tfm);
-	return ret;
-}

Let's just say, theoretically, that there was some future hardware that also supported SHA384. There doesn't seem to be a SHA-384 library API.

Quoted 2 lines

Would you leave the crypto_shash() in place if that were to be happening? Theoretically of course.

I don't see any reason why SHA-384 could not be added in order to support a CPU feature.

BR, Jarkko