Re: [PATCH v2 0/2] support sign module with SM2-with-SM3 algorithm
ping. Thanks, Tianjia On 3/24/21 8:15 PM, Tianjia Zhang wrote: The kernel module signature supports the option to use the SM3 secure hash (OSCCA GM/T 0004-2012 SM3). SM2 and SM3 always appear in pairs. The former is used for signing and the latter is used for hash calculation. To sign a kernel module, first, prepare openssl 3.0.0 alpha6 and a configuration file openssl.cnf with the following content: [ req ] default_bits = 2048 distinguished_name = req_distinguished_name prompt = no string_mask = utf8only x509_extensions = v3_req [ req_distinguished_name ] C = CN ST = HangZhou L = foo O = Test OU = Test CN = Test key emailAddress = t...@foo.com [ v3_req ] basicConstraints=critical,CA:FALSE keyUsage=digitalSignature subjectKeyIdentifier=hash authorityKeyIdentifier=keyid:always Then we can use the following method to sign module with SM2-with-SM3 algorithm combination: # generate CA key and self-signed CA certificate openssl ecparam -genkey -name SM2 -text -out ca.key openssl req -new -x509 -days 3650 -key ca.key \ -sm3 -sigopt "distid:1234567812345678" \ -subj "/O=testCA/OU=testCA/CN=testCA/emailAddress=c...@foo.com" \ -config openssl.cnf -out ca.crt # generate SM2 private key and sign request openssl ecparam -genkey -name SM2 -text -out private.pem openssl req -new -key private.pem -config openssl.cnf \ -sm3 -sigopt "distid:1234567812345678" -out csr.pem # generate SM2-with-SM3 certificate signed by CA openssl x509 -req -days 3650 -sm3 -in csr.pem \ -sigopt "distid:1234567812345678" \ -vfyopt "distid:1234567812345678" \ -CA ca.crt -CAkey ca.key -CAcreateserial \ -extfile openssl.cnf -extensions v3_req \ -out cert.pem # sign module with SM2-with-SM3 algorithm sign-file sm3 private.pem cert.pem test.ko test.ko.signed At this point, we should built the CA certificate into the kernel, and then we can load the SM2-with-SM3 signed module normally. --- v2 change: - split one patch into twos. - richer commit log. Tianjia Zhang (2): pkcs7: make parser enable SM2 and SM3 algorithms combination init/Kconfig: support sign module with SM2-with-SM3 algorithm Documentation/admin-guide/module-signing.rst | 5 +++-- crypto/asymmetric_keys/pkcs7_parser.c| 7 +++ init/Kconfig | 5 + 3 files changed, 15 insertions(+), 2 deletions(-)
Re: [PATCH] crypto: sm3 - use the more precise type u32 instead of unsigned int
On 3/26/21 5:38 PM, Gilad Ben-Yossef wrote: Hi, Thank you for the patch! On Fri, Mar 26, 2021 at 5:21 AM Tianjia Zhang wrote: In the process of calculating the hash, use the more accurate type 'u32' instead of the original 'unsigned int' to avoid ambiguity. I don't think there is any ambiguity here, as both forms are always the same size. Generally, I tend to use the convention of using 'u32' as denoting variables where the size is meaningful - e.g. mathematical operations that are defined in the standard on 32 bit buffers, versus using plain 'int' types where it isn't - e.g. loop counters etc. Having said that, even under my own definition possibly the w and wt arrays in sm3_trandform() should be changed to u32. I don't object to changing those if it bugs you :-) Cheers, Gilad Thanks for your opinions. This is just to make the form more uniform. This is not a mistake. If it is not necessary, just ignore this modification. Best regards, Tianjia
[PATCH] crypto: sm3 - use the more precise type u32 instead of unsigned int
In the process of calculating the hash, use the more accurate type 'u32' instead of the original 'unsigned int' to avoid ambiguity. Signed-off-by: Tianjia Zhang --- crypto/sm3_generic.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crypto/sm3_generic.c b/crypto/sm3_generic.c index 193c4584bd00..562e96f92f64 100644 --- a/crypto/sm3_generic.c +++ b/crypto/sm3_generic.c @@ -36,17 +36,17 @@ static inline u32 p1(u32 x) return x ^ rol32(x, 15) ^ rol32(x, 23); } -static inline u32 ff(unsigned int n, u32 a, u32 b, u32 c) +static inline u32 ff(u32 n, u32 a, u32 b, u32 c) { return (n < 16) ? (a ^ b ^ c) : ((a & b) | (a & c) | (b & c)); } -static inline u32 gg(unsigned int n, u32 e, u32 f, u32 g) +static inline u32 gg(u32 n, u32 e, u32 f, u32 g) { return (n < 16) ? (e ^ f ^ g) : ((e & f) | ((~e) & g)); } -static inline u32 t(unsigned int n) +static inline u32 t(u32 n) { return (n < 16) ? SM3_T1 : SM3_T2; } @@ -54,7 +54,7 @@ static inline u32 t(unsigned int n) static void sm3_expand(u32 *t, u32 *w, u32 *wt) { int i; - unsigned int tmp; + u32 tmp; /* load the input */ for (i = 0; i <= 15; i++) @@ -123,8 +123,8 @@ static void sm3_compress(u32 *w, u32 *wt, u32 *m) static void sm3_transform(struct sm3_state *sst, u8 const *src) { - unsigned int w[68]; - unsigned int wt[64]; + u32 w[68]; + u32 wt[64]; sm3_expand((u32 *)src, w, wt); sm3_compress(w, wt, sst->state); -- 2.19.1.3.ge56e4f7
[PATCH v2 2/2] init/Kconfig: support sign module with SM2-with-SM3 algorithm
The kernel module signature supports the option to use the SM3 secure hash (OSCCA GM/T 0004-2012 SM3). SM2 and SM3 always appear in pairs. The former is used for signing and the latter is used for hash calculation. To sign a kernel module, first, prepare a configuration file openssl.cnf with the following content: [ req ] default_bits = 2048 distinguished_name = req_distinguished_name prompt = no string_mask = utf8only x509_extensions = v3_req [ req_distinguished_name ] C = CN ST = HangZhou L = foo O = Test OU = Test CN = Test key emailAddress = t...@foo.com [ v3_req ] basicConstraints=critical,CA:FALSE keyUsage=digitalSignature subjectKeyIdentifier=hash authorityKeyIdentifier=keyid:always Then we can use the following method to sign module with SM2-with-SM3 algorithm combination: # generate CA key and self-signed CA certificate openssl ecparam -genkey -name SM2 -text -out ca.key openssl req -new -x509 -days 3650 -key ca.key \ -sm3 -sigopt "distid:1234567812345678" \ -subj "/O=testCA/OU=testCA/CN=testCA/emailAddress=c...@foo.com" \ -config openssl.cnf -out ca.crt # generate SM2 private key and sign request openssl ecparam -genkey -name SM2 -text -out private.pem openssl req -new -key private.pem -config openssl.cnf \ -sm3 -sigopt "distid:1234567812345678" -out csr.pem # generate SM2-with-SM3 certificate signed by CA openssl x509 -req -days 3650 -sm3 -in csr.pem \ -sigopt "distid:1234567812345678" \ -vfyopt "distid:1234567812345678" \ -CA ca.crt -CAkey ca.key -CAcreateserial \ -extfile openssl.cnf -extensions v3_req \ -out cert.pem # sign module with SM2-with-SM3 algorithm sign-file sm3 private.pem cert.pem test.ko test.ko.signed At this point, we should built the CA certificate into the kernel, and then we can load the SM2-with-SM3 signed module normally. Signed-off-by: Tianjia Zhang --- Documentation/admin-guide/module-signing.rst | 5 +++-- init/Kconfig | 5 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst index 7d7c7c8a545c..8d8980808b5b 100644 --- a/Documentation/admin-guide/module-signing.rst +++ b/Documentation/admin-guide/module-signing.rst @@ -30,8 +30,8 @@ This facility uses X.509 ITU-T standard certificates to encode the public keys involved. The signatures are not themselves encoded in any industrial standard type. The facility currently only supports the RSA public key encryption standard (though it is pluggable and permits others to be used). The possible -hash algorithms that can be used are SHA-1, SHA-224, SHA-256, SHA-384, and -SHA-512 (the algorithm is selected by data in the signature). +hash algorithms that can be used are SHA-1, SHA-224, SHA-256, SHA-384, SHA-512, +and SM3 (the algorithm is selected by data in the signature). == @@ -86,6 +86,7 @@ This has a number of options available: ``CONFIG_MODULE_SIG_SHA256``:menuselection:`Sign modules with SHA-256` ``CONFIG_MODULE_SIG_SHA384``:menuselection:`Sign modules with SHA-384` ``CONFIG_MODULE_SIG_SHA512``:menuselection:`Sign modules with SHA-512` + ``CONFIG_MODULE_SIG_SM3`` :menuselection:`Sign modules with SM3` === == The algorithm selected here will also be built into the kernel (rather diff --git a/init/Kconfig b/init/Kconfig index 5f5c776ef192..fed9236078e4 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -2202,6 +2202,10 @@ config MODULE_SIG_SHA512 bool "Sign modules with SHA-512" select CRYPTO_SHA512 +config MODULE_SIG_SM3 + bool "Sign modules with SM3" + select CRYPTO_SM3 + endchoice config MODULE_SIG_HASH @@ -2212,6 +2216,7 @@ config MODULE_SIG_HASH default "sha256" if MODULE_SIG_SHA256 default "sha384" if MODULE_SIG_SHA384 default "sha512" if MODULE_SIG_SHA512 + default "sm3" if MODULE_SIG_SM3 config MODULE_COMPRESS bool "Compress modules on installation" -- 2.19.1.3.ge56e4f7
[PATCH v2 0/2] support sign module with SM2-with-SM3 algorithm
The kernel module signature supports the option to use the SM3 secure hash (OSCCA GM/T 0004-2012 SM3). SM2 and SM3 always appear in pairs. The former is used for signing and the latter is used for hash calculation. To sign a kernel module, first, prepare openssl 3.0.0 alpha6 and a configuration file openssl.cnf with the following content: [ req ] default_bits = 2048 distinguished_name = req_distinguished_name prompt = no string_mask = utf8only x509_extensions = v3_req [ req_distinguished_name ] C = CN ST = HangZhou L = foo O = Test OU = Test CN = Test key emailAddress = t...@foo.com [ v3_req ] basicConstraints=critical,CA:FALSE keyUsage=digitalSignature subjectKeyIdentifier=hash authorityKeyIdentifier=keyid:always Then we can use the following method to sign module with SM2-with-SM3 algorithm combination: # generate CA key and self-signed CA certificate openssl ecparam -genkey -name SM2 -text -out ca.key openssl req -new -x509 -days 3650 -key ca.key \ -sm3 -sigopt "distid:1234567812345678" \ -subj "/O=testCA/OU=testCA/CN=testCA/emailAddress=c...@foo.com" \ -config openssl.cnf -out ca.crt # generate SM2 private key and sign request openssl ecparam -genkey -name SM2 -text -out private.pem openssl req -new -key private.pem -config openssl.cnf \ -sm3 -sigopt "distid:1234567812345678" -out csr.pem # generate SM2-with-SM3 certificate signed by CA openssl x509 -req -days 3650 -sm3 -in csr.pem \ -sigopt "distid:1234567812345678" \ -vfyopt "distid:1234567812345678" \ -CA ca.crt -CAkey ca.key -CAcreateserial \ -extfile openssl.cnf -extensions v3_req \ -out cert.pem # sign module with SM2-with-SM3 algorithm sign-file sm3 private.pem cert.pem test.ko test.ko.signed At this point, we should built the CA certificate into the kernel, and then we can load the SM2-with-SM3 signed module normally. --- v2 change: - split one patch into twos. - richer commit log. Tianjia Zhang (2): pkcs7: make parser enable SM2 and SM3 algorithms combination init/Kconfig: support sign module with SM2-with-SM3 algorithm Documentation/admin-guide/module-signing.rst | 5 +++-- crypto/asymmetric_keys/pkcs7_parser.c| 7 +++ init/Kconfig | 5 + 3 files changed, 15 insertions(+), 2 deletions(-) -- 2.19.1.3.ge56e4f7
[PATCH v2 1/2] pkcs7: make parser enable SM2 and SM3 algorithms combination
Support parsing the message signature of the SM2 and SM3 algorithm combination. This group of algorithms has been well supported. One of the main users is module signature verification. Signed-off-by: Tianjia Zhang --- crypto/asymmetric_keys/pkcs7_parser.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c index 967329e0a07b..6cf6c4552c11 100644 --- a/crypto/asymmetric_keys/pkcs7_parser.c +++ b/crypto/asymmetric_keys/pkcs7_parser.c @@ -248,6 +248,9 @@ int pkcs7_sig_note_digest_algo(void *context, size_t hdrlen, case OID_sha224: ctx->sinfo->sig->hash_algo = "sha224"; break; + case OID_sm3: + ctx->sinfo->sig->hash_algo = "sm3"; + break; default: printk("Unsupported digest algo: %u\n", ctx->last_oid); return -ENOPKG; @@ -269,6 +272,10 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen, ctx->sinfo->sig->pkey_algo = "rsa"; ctx->sinfo->sig->encoding = "pkcs1"; break; + case OID_SM2_with_SM3: + ctx->sinfo->sig->pkey_algo = "sm2"; + ctx->sinfo->sig->encoding = "raw"; + break; default: printk("Unsupported pkey algo: %u\n", ctx->last_oid); return -ENOPKG; -- 2.19.1.3.ge56e4f7
[tip: x86/sgx] selftests/sgx: Use getauxval() to simplify test code
The following commit has been merged into the x86/sgx branch of tip: Commit-ID: f33dece70e11ce82a09cb1ea2d7c32347b82c67e Gitweb: https://git.kernel.org/tip/f33dece70e11ce82a09cb1ea2d7c32347b82c67e Author:Tianjia Zhang AuthorDate:Sun, 14 Mar 2021 19:16:21 +08:00 Committer: Borislav Petkov CommitterDate: Wed, 24 Mar 2021 10:59:09 +01:00 selftests/sgx: Use getauxval() to simplify test code Use the library function getauxval() instead of a custom function to get the base address of the vDSO. [ bp: Massage commit message. ] Signed-off-by: Tianjia Zhang Signed-off-by: Borislav Petkov Reviewed-by: Jarkko Sakkinen Acked-by: Shuah Khan Link: https://lkml.kernel.org/r/20210314111621.68428-1-tianjia.zh...@linux.alibaba.com --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index b117bb8..d304a40 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)getauxval(AT_SYSINFO_EHDR); if (!addr) goto err;
Re: [PATCH] init/Kconfig: Support sign module with SM3 hash algorithm
Hi, On 3/24/21 6:14 AM, Ard Biesheuvel wrote: On Tue, 23 Mar 2021 at 09:36, Tianjia Zhang wrote: The kernel module signature supports the option to use the SM3 secure hash (OSCCA GM/T 0004-2012 SM3). Signed-off-by: Tianjia Zhang A secure hash is not the same as a signature. Looking at the patch, the asymmetric algorithm that is used to sign the SM3 digest is SM2, is that correct? How does one create such signed modules? In any case, please provide more context in the commit log on how this is intended to be used. Sorry for the trouble you have caused. You are right. SM2 and SM3 always appear in pairs. The former is used for signatures and the latter is used for hashing algorithms. I will add this information in the next version. It seems This is more appropriate to split into two patches. Best regards, Tianjia
Re: [PATCH] init/Kconfig: Support sign module with SM3 hash algorithm
Hi, On 3/24/21 12:43 AM, Randy Dunlap wrote: On 3/23/21 1:35 AM, Tianjia Zhang wrote: The kernel module signature supports the option to use the SM3 secure hash (OSCCA GM/T 0004-2012 SM3). Signed-off-by: Tianjia Zhang --- Documentation/admin-guide/module-signing.rst | 5 +++-- crypto/asymmetric_keys/pkcs7_parser.c| 7 +++ init/Kconfig | 5 + 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index 5f5c776ef192..fed9236078e4 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -2202,6 +2202,10 @@ config MODULE_SIG_SHA512 bool "Sign modules with SHA-512" select CRYPTO_SHA512 +config MODULE_SIG_SM3 + bool "Sign modules with SM3" + select CRYPTO_SM3 + endchoice config MODULE_SIG_HASH @@ -2212,6 +2216,7 @@ config MODULE_SIG_HASH default "sha256" if MODULE_SIG_SHA256 default "sha384" if MODULE_SIG_SHA384 default "sha512" if MODULE_SIG_SHA512 + default "sm3" if MODULE_SIG_SM3 config MODULE_COMPRESS bool "Compress modules on installation" checkpatch tells me: WARNING: please write a paragraph that describes the config symbol fully #74: FILE: init/Kconfig:2205: +config MODULE_SIG_SM3 so yes, it should have some help text there. thanks. I noticed, but this is just a list of algorithms, this warning can be ignored. Best regards, Tianjia
Re: [PATCH v6] selftests/x86: Use getauxval() to simplify the code in sgx
Hi, On 3/24/21 2:51 AM, Borislav Petkov wrote: On Tue, Mar 23, 2021 at 11:08:25AM +0800, Tianjia Zhang wrote: Take time to look at this. A "please" wouldn't hurt. I'm very sorry that my mistake caused your hurt. Please take time to look at this, which tree this should be picked? Best regards, Tianjia
[PATCH] init/Kconfig: Support sign module with SM3 hash algorithm
The kernel module signature supports the option to use the SM3 secure hash (OSCCA GM/T 0004-2012 SM3). Signed-off-by: Tianjia Zhang --- Documentation/admin-guide/module-signing.rst | 5 +++-- crypto/asymmetric_keys/pkcs7_parser.c| 7 +++ init/Kconfig | 5 + 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst index 7d7c7c8a545c..8d8980808b5b 100644 --- a/Documentation/admin-guide/module-signing.rst +++ b/Documentation/admin-guide/module-signing.rst @@ -30,8 +30,8 @@ This facility uses X.509 ITU-T standard certificates to encode the public keys involved. The signatures are not themselves encoded in any industrial standard type. The facility currently only supports the RSA public key encryption standard (though it is pluggable and permits others to be used). The possible -hash algorithms that can be used are SHA-1, SHA-224, SHA-256, SHA-384, and -SHA-512 (the algorithm is selected by data in the signature). +hash algorithms that can be used are SHA-1, SHA-224, SHA-256, SHA-384, SHA-512, +and SM3 (the algorithm is selected by data in the signature). == @@ -86,6 +86,7 @@ This has a number of options available: ``CONFIG_MODULE_SIG_SHA256``:menuselection:`Sign modules with SHA-256` ``CONFIG_MODULE_SIG_SHA384``:menuselection:`Sign modules with SHA-384` ``CONFIG_MODULE_SIG_SHA512``:menuselection:`Sign modules with SHA-512` + ``CONFIG_MODULE_SIG_SM3`` :menuselection:`Sign modules with SM3` === == The algorithm selected here will also be built into the kernel (rather diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c index 967329e0a07b..6cf6c4552c11 100644 --- a/crypto/asymmetric_keys/pkcs7_parser.c +++ b/crypto/asymmetric_keys/pkcs7_parser.c @@ -248,6 +248,9 @@ int pkcs7_sig_note_digest_algo(void *context, size_t hdrlen, case OID_sha224: ctx->sinfo->sig->hash_algo = "sha224"; break; + case OID_sm3: + ctx->sinfo->sig->hash_algo = "sm3"; + break; default: printk("Unsupported digest algo: %u\n", ctx->last_oid); return -ENOPKG; @@ -269,6 +272,10 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen, ctx->sinfo->sig->pkey_algo = "rsa"; ctx->sinfo->sig->encoding = "pkcs1"; break; + case OID_SM2_with_SM3: + ctx->sinfo->sig->pkey_algo = "sm2"; + ctx->sinfo->sig->encoding = "raw"; + break; default: printk("Unsupported pkey algo: %u\n", ctx->last_oid); return -ENOPKG; diff --git a/init/Kconfig b/init/Kconfig index 5f5c776ef192..fed9236078e4 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -2202,6 +2202,10 @@ config MODULE_SIG_SHA512 bool "Sign modules with SHA-512" select CRYPTO_SHA512 +config MODULE_SIG_SM3 + bool "Sign modules with SM3" + select CRYPTO_SM3 + endchoice config MODULE_SIG_HASH @@ -2212,6 +2216,7 @@ config MODULE_SIG_HASH default "sha256" if MODULE_SIG_SHA256 default "sha384" if MODULE_SIG_SHA384 default "sha512" if MODULE_SIG_SHA512 + default "sm3" if MODULE_SIG_SM3 config MODULE_COMPRESS bool "Compress modules on installation" -- 2.19.1.3.ge56e4f7
Re: [PATCH v6] selftests/x86: Use getauxval() to simplify the code in sgx
Hi, On 3/15/21 9:02 PM, Jarkko Sakkinen wrote: On Sun, Mar 14, 2021 at 07:16:21PM +0800, Tianjia Zhang wrote: Simplify the sgx code implemntation by using library function getauxval() instead of a custom function to get the base address of vDSO. Signed-off-by: Tianjia Zhang Reviewed-by: Jarkko Sakkinen Acked-by: Shuah Khan Shuah, Boris, which tree this should be picked? /Jarkko Take time to look at this. Best regards, Tianjia
[PATCH] sign-file: Fix confusing error messages
When an error occurs, use errx() instead of err() to display the error message, because openssl has its own error record. When an error occurs, errno will not be changed, while err() displays the errno error message. It will cause confusion. For example, when CMS_add1_signer() fails, the following message will appear: sign-file: CMS_add1_signer: Success errx() ignores errno and does not cause such issue. Signed-off-by: Tianjia Zhang --- scripts/sign-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/sign-file.c b/scripts/sign-file.c index fbd34b8e8f57..37d8760cb0d1 100644 --- a/scripts/sign-file.c +++ b/scripts/sign-file.c @@ -107,7 +107,7 @@ static void drain_openssl_errors(void) bool __cond = (cond); \ display_openssl_errors(__LINE__); \ if (__cond) { \ - err(1, fmt, ## __VA_ARGS__);\ + errx(1, fmt, ## __VA_ARGS__); \ } \ } while(0) -- 2.19.1.3.ge56e4f7
[PATCH v6] selftests/x86: Use getauxval() to simplify the code in sgx
Simplify the sgx code implemntation by using library function getauxval() instead of a custom function to get the base address of vDSO. Signed-off-by: Tianjia Zhang Reviewed-by: Jarkko Sakkinen Acked-by: Shuah Khan --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 724cec700926..5167505fbb46 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)getauxval(AT_SYSINFO_EHDR); if (!addr) goto err; -- 2.19.1.3.ge56e4f7
Re: [PATCH] selftests/sgx: fix EINIT failure dueto SGX_INVALID_SIGNATURE
On 3/11/21 11:42 AM, Jarkko Sakkinen wrote: On Thu, Mar 11, 2021 at 10:47:50AM +0800, Jia Zhang wrote: On 2021/3/11 上午5:39, Jarkko Sakkinen wrote: On Wed, Mar 10, 2021 at 08:44:44PM +0800, Jia Zhang wrote: On 2021/3/2 下午9:47, Jarkko Sakkinen wrote: On Mon, Mar 01, 2021 at 09:54:37PM -0800, Andy Lutomirski wrote: On Mon, Mar 1, 2021 at 9:06 PM Tianjia Zhang wrote: On 3/1/21 5:54 PM, Jarkko Sakkinen wrote: On Mon, Mar 01, 2021 at 01:18:36PM +0800, Tianjia Zhang wrote: q2 is not always 384-byte length. Sometimes it only has 383-byte. What does determine this? In this case, the valid portion of q2 is reordered reversely for little endian order, and the remaining portion is filled with zero. I'm presuming that you want to say "In this case, q2 needs to be reversed because...". I'm lacking these details: 1. Why the length of Q2 can vary? 2. Why reversing the bytes is the correct measure to counter-measure this variation? /Jarkko When use openssl to generate a key instead of using the built-in sign_key.pem, there is a probability that will encounter this problem. Here is a problematic key I encountered. The calculated q1 and q2 of this key are both 383 bytes, If the length is not processed, the hardware signature will fail. Presumably the issue is that some keys have parameters that have enough leading 0 bits to be effectively shorter. The openssl API (and, sadly, a bunch of the ASN.1 stuff) treats these parameters as variable-size integers. But the test uses a static key. It used to generate a key on fly but IMO even though the test code, it comes from the linux kernel, meaning that its quality has a certain guarantee and it is a good reference, so the test code still needs to ensure its correctness. Hmm... what is working incorrectly then? In current implementation, it is working well, after all the static key can derive the full 384-byte length of q1 and q2. As mentioned above, if someone refers to the design of signing tool from selftest code, it is quite possible that the actual implementation will use dynamical or external signing key deriving shorter q1 and/or q2 in length. A self-test needs is not meant to be generic to be directly used in 3rd party code. With the current key there is not issue => there is no issue. For keys generated on fly, self-test does not work properly, this experience is really worse. Best regards, Tianjia
Re: [PATCH v9 2/9] x509: Detect sm2 keys by their parameters OID
Hi, On 3/5/21 11:04 PM, Stefan Berger wrote: On 3/5/21 2:37 AM, Tianjia Zhang wrote: Hi, On 3/4/21 7:46 AM, Stefan Berger wrote: Tianjia, can you say whether SM2 support works for you before and after applying this patch? I cannot verify it with an sm2 key I have created using a sequence of commands like this: > modprobe sm2_generic > id=$(keyctl newring test @u) > keyctl padd asymmetric "" $id < sm2.der add_key: Key was rejected by service > keyctl padd asymmetric "" $id < eckeys/cert-prime192v1-0.der 88506426 The sm2 key is reject but the pime192v1 key works just fine. SM2 support neither worked for me before nor after this patch here. The difference is that before it returned 'add_key: Package not installed'. This is my sm2 cert: > base64 < sm2.der MIIBbzCCARWgAwIBAgIUfqwndeAy7reymWLwvCHOgYPU2YUwCgYIKoZIzj0EAwIwDTELMAkGA1UE AwwCbWUwHhcNMjEwMTI0MTgwNjQ3WhcNMjIwMTI0MTgwNjQ3WjANMQswCQYDVQQDDAJtZTBZMBMG ByqGSM49AgEGCCqBHM9VAYItA0IABEtiMaczdk46MEugmOsY/u+puf5qoi7JdLd/w3VpdixvDd26 vrxLKL7lCTVn5w3a07G7QB1dgdMDpzIRgWrVXC6jUzBRMB0GA1UdDgQWBBSxOVnE7ihvTb6Nczb4 /mow+HIc9TAfBgNVHSMEGDAWgBSxOVnE7ihvTb6Nczb4/mow+HIc9TAPBgNVHRMBAf8EBTADAQH/ MAoGCCqGSM49BAMCA0gAMEUCIE1kiji2ABUy663NANe0iCPjCeeqg02Yk4b3K+Ci/Qh4AiEA/cFB eJEVklyveRMvuTP7BN7FG4U8iRdtedjiX+YrNio= Regards, Stefan Yes, it works fine here. Your test method may be wrong. First of all, the certificate looks wrong, I don’t know if it is not sent completely. Secondly, the SM2 algorithm must be compiled with builtin. There will be a problem when it is compiled into a module. This is a restriction for SM2 signature with Za. you may refer to this discussion: https://lkml.org/lkml/2021/1/12/1736 In addition, give you a self-signed root certificate for my test: -BEGIN CERTIFICATE- MIICLjCCAdWgAwIBAgIUEoozP6LzMYWh4gCpcWlzsUyfgsIwCgYIKoEcz1UBg3Uw bTELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdTMQswCQYDVQQHDAJHdDENMAsGA1UE CgwEYmFiYTELMAkGA1UECwwCT1MxCzAJBgNVBAMMAmNhMRswGQYJKoZIhvcNAQkB FgxjYUB3b3JsZC5jb20wHhcNMjAwNDE1MTE1NDA3WhcNMzAwNDEzMTE1NDA3WjBt MQswCQYDVQQGEwJDTjELMAkGA1UECAwCR1MxCzAJBgNVBAcMAkd0MQ0wCwYDVQQK DARiYWJhMQswCQYDVQQLDAJPUzELMAkGA1UEAwwCY2ExGzAZBgkqhkiG9w0BCQEW DGNhQHdvcmxkLmNvbTBZMBMGByqGSM49AgEGCCqBHM9VAYItA0IABMTGRiHezKm5 MiKHlyfa5Bv5jLxge/WRRG0nLNsZx1yf0XQTQBR/tFFjPGePEr7+Fa1CPgYpXExx i44coYMmQT6jUzBRMB0GA1UdDgQWBBSjd9GWIe98Ll9J0dquxgCktp9DrTAfBgNV HSMEGDAWgBSjd9GWIe98Ll9J0dquxgCktp9DrTAPBgNVHRMBAf8EBTADAQH/MAoG CCqBHM9VAYN1A0cAMEQCIAvLWIfGFq85u/vVMLc5H1D/DnrNS0VhSkQA4daRO4tc AiABbeWENcQZDZLWTuqG9P2KDPOoNqV/QV/+0XjMAVblhg== -END CERTIFICATE- If you can, please add: Tested-by: Tianjia Zhang good luck! It works with your certificate! When I create the sm2 with OpenSSL on Ubuntu, which seems to have sm2 support, or so one may think, I get this type of signature: I cannot create it with sm3, if that's how this is supposed to be signed: > # openssl req -x509 -sm3 -newkey ec -pkeyopt ec_paramgen_curve:sm2 -keyout sm2key.pem -days 365 -subj '/CN=test' -nodes -outform der -out sm2.der parameter error "ec_paramgen_curve:sm2" 140735899258064:error:100AE081:elliptic curve routines:EC_GROUP_new_by_curve_name:unknown group:crypto/ec/ec_curve.c:418: 140735899258064:error:100C508D:elliptic curve routines:pkey_ec_ctrl:invalid curve:crypto/ec/ec_pmeth.c:231: Using sha256 instead, which is the mistake: > openssl req -x509 -sha256 -newkey ec -pkeyopt ec_paramgen_curve:sm2 -keyout sm2key.pem -days 365 -subj '/CN=test' -nodes -outform der -out sm2-2nd.der > openssl x509 -inform der -in sm2-2nd.der -noout -text [...] Signature Algorithm: ecdsa-with-SHA256 30:45:02:20:05:72:15:b0:e8:f5:5a:27:d2:fb:f9:de:de:35: 05:b2:76:8a:6f:84:c1:54:db:c2:38:8c:d2:64:8b:67:23:01: 02:21:00:97:77:9e:42:fa:41:3d:d4:81:5e:5b:ad:9e:56:ad: 46:fc:5e:94:92:a6:07:2d:af:62:d2:2d:39:7b:71:f1:4a Yours has this type of signature: Signature Algorithm: 1.2.156.10197.1.501 30:44:02:20:0b:cb:58:87:c6:16:af:39:bb:fb:d5:30:b7:39: 1f:50:ff:0e:7a:cd:4b:45:61:4a:44:00:e1:d6:91:3b:8b:5c: 02:20:01:6d:e5:84:35:c4:19:0d:92:d6:4e:ea:86:f4:fd:8a: 0c:f3:a8:36:a5:7f:41:5f:fe:d1:78:cc:01:56:e5:86 Thanks anyway! Stefan I guess it may be that your openssl version is too low. At present, only openssl 3.0.0 (still in the alpha stage and not yet officially released) can support the certificate of the SM2-with-SM3 algorithm combination. This is the command I used in openssl-3.0.0-alpha6: openssl ecparam -genkey -name SM2 -text -out ca.key openssl req -new \ -x509 -days 3650 \ -sm3 -sigopt "distid:1234567812345678" \ -key ca.key \ -out ca.crt \ -subj "/C=CN/ST=GS/L=Gt/O=baba/OU=OS/CN=ca/emailAddress=c...@world.com"
Re: [PATCH v10 5/9] x509: Detect sm2 keys by their parameters OID
Hi, On 3/5/21 8:51 AM, Stefan Berger wrote: From: Stefan Berger Detect whether a key is an sm2 type of key by its OID in the parameters array rather than assuming that everything under OID_id_ecPublicKey is sm2, which is not the case. Cc: David Howells Cc: keyri...@vger.kernel.org Signed-off-by: Stefan Berger Reviewed-by: Tianjia Zhang --- crypto/asymmetric_keys/x509_cert_parser.c | 12 +++- include/linux/oid_registry.h | 1 + lib/oid_registry.c| 13 + 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 52c9b455fc7d..1621ceaf5c95 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -459,6 +459,7 @@ int x509_extract_key_data(void *context, size_t hdrlen, const void *value, size_t vlen) { struct x509_parse_context *ctx = context; + enum OID oid; ctx->key_algo = ctx->last_oid; switch (ctx->last_oid) { @@ -470,7 +471,16 @@ int x509_extract_key_data(void *context, size_t hdrlen, ctx->cert->pub->pkey_algo = "ecrdsa"; break; case OID_id_ecPublicKey: - ctx->cert->pub->pkey_algo = "sm2"; + if (parse_OID(ctx->params, ctx->params_size, ) != 0) + return -EBADMSG; + + switch (oid) { + case OID_sm2: + ctx->cert->pub->pkey_algo = "sm2"; + break; + default: + return -ENOPKG; + } break; default: return -ENOPKG; diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index b504e2f36b25..f32d91895e4d 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -121,6 +121,7 @@ enum OID { }; extern enum OID look_up_OID(const void *data, size_t datasize); +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); extern int sprint_oid(const void *, size_t, char *, size_t); extern int sprint_OID(enum OID, char *, size_t); diff --git a/lib/oid_registry.c b/lib/oid_registry.c index f7ad43f28579..508e0b34b5f0 100644 --- a/lib/oid_registry.c +++ b/lib/oid_registry.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "oid_registry_data.c" MODULE_DESCRIPTION("OID Registry"); @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) } EXPORT_SYMBOL_GPL(look_up_OID); +int parse_OID(const void *data, size_t datasize, enum OID *oid) +{ + const unsigned char *v = data; + + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) + return -EBADMSG; + + *oid = look_up_OID(data + 2, datasize - 2); + return 0; +} +EXPORT_SYMBOL_GPL(parse_OID); + /* * sprint_OID - Print an Object Identifier into a buffer * @data: The encoded OID to print Tested-by: Tianjia Zhang Best regards, Tianjia
Re: [PATCH v9 2/9] x509: Detect sm2 keys by their parameters OID
Hi, On 3/4/21 7:46 AM, Stefan Berger wrote: Tianjia, can you say whether SM2 support works for you before and after applying this patch? I cannot verify it with an sm2 key I have created using a sequence of commands like this: > modprobe sm2_generic > id=$(keyctl newring test @u) > keyctl padd asymmetric "" $id < sm2.der add_key: Key was rejected by service > keyctl padd asymmetric "" $id < eckeys/cert-prime192v1-0.der 88506426 The sm2 key is reject but the pime192v1 key works just fine. SM2 support neither worked for me before nor after this patch here. The difference is that before it returned 'add_key: Package not installed'. This is my sm2 cert: > base64 < sm2.der MIIBbzCCARWgAwIBAgIUfqwndeAy7reymWLwvCHOgYPU2YUwCgYIKoZIzj0EAwIwDTELMAkGA1UE AwwCbWUwHhcNMjEwMTI0MTgwNjQ3WhcNMjIwMTI0MTgwNjQ3WjANMQswCQYDVQQDDAJtZTBZMBMG ByqGSM49AgEGCCqBHM9VAYItA0IABEtiMaczdk46MEugmOsY/u+puf5qoi7JdLd/w3VpdixvDd26 vrxLKL7lCTVn5w3a07G7QB1dgdMDpzIRgWrVXC6jUzBRMB0GA1UdDgQWBBSxOVnE7ihvTb6Nczb4 /mow+HIc9TAfBgNVHSMEGDAWgBSxOVnE7ihvTb6Nczb4/mow+HIc9TAPBgNVHRMBAf8EBTADAQH/ MAoGCCqGSM49BAMCA0gAMEUCIE1kiji2ABUy663NANe0iCPjCeeqg02Yk4b3K+Ci/Qh4AiEA/cFB eJEVklyveRMvuTP7BN7FG4U8iRdtedjiX+YrNio= Regards, Stefan Yes, it works fine here. Your test method may be wrong. First of all, the certificate looks wrong, I don’t know if it is not sent completely. Secondly, the SM2 algorithm must be compiled with builtin. There will be a problem when it is compiled into a module. This is a restriction for SM2 signature with Za. you may refer to this discussion: https://lkml.org/lkml/2021/1/12/1736 In addition, give you a self-signed root certificate for my test: -BEGIN CERTIFICATE- MIICLjCCAdWgAwIBAgIUEoozP6LzMYWh4gCpcWlzsUyfgsIwCgYIKoEcz1UBg3Uw bTELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkdTMQswCQYDVQQHDAJHdDENMAsGA1UE CgwEYmFiYTELMAkGA1UECwwCT1MxCzAJBgNVBAMMAmNhMRswGQYJKoZIhvcNAQkB FgxjYUB3b3JsZC5jb20wHhcNMjAwNDE1MTE1NDA3WhcNMzAwNDEzMTE1NDA3WjBt MQswCQYDVQQGEwJDTjELMAkGA1UECAwCR1MxCzAJBgNVBAcMAkd0MQ0wCwYDVQQK DARiYWJhMQswCQYDVQQLDAJPUzELMAkGA1UEAwwCY2ExGzAZBgkqhkiG9w0BCQEW DGNhQHdvcmxkLmNvbTBZMBMGByqGSM49AgEGCCqBHM9VAYItA0IABMTGRiHezKm5 MiKHlyfa5Bv5jLxge/WRRG0nLNsZx1yf0XQTQBR/tFFjPGePEr7+Fa1CPgYpXExx i44coYMmQT6jUzBRMB0GA1UdDgQWBBSjd9GWIe98Ll9J0dquxgCktp9DrTAfBgNV HSMEGDAWgBSjd9GWIe98Ll9J0dquxgCktp9DrTAPBgNVHRMBAf8EBTADAQH/MAoG CCqBHM9VAYN1A0cAMEQCIAvLWIfGFq85u/vVMLc5H1D/DnrNS0VhSkQA4daRO4tc AiABbeWENcQZDZLWTuqG9P2KDPOoNqV/QV/+0XjMAVblhg== -END CERTIFICATE- If you can, please add: Tested-by: Tianjia Zhang good luck! Tianjia
Re: [PATCH] selftests/sgx: fix EINIT failure dueto SGX_INVALID_SIGNATURE
On 3/2/21 8:51 PM, Jarkko Sakkinen wrote: Nit: "due to" Start with capital letter "Fix" Will do in the next patch. On Tue, Mar 02, 2021 at 01:06:52PM +0800, Tianjia Zhang wrote: On 3/1/21 5:54 PM, Jarkko Sakkinen wrote: On Mon, Mar 01, 2021 at 01:18:36PM +0800, Tianjia Zhang wrote: q2 is not always 384-byte length. Sometimes it only has 383-byte. What does determine this? In this case, the valid portion of q2 is reordered reversely for little endian order, and the remaining portion is filled with zero. I'm presuming that you want to say "In this case, q2 needs to be reversed because...". I'm lacking these details: 1. Why the length of Q2 can vary? 2. Why reversing the bytes is the correct measure to counter-measure this variation? /Jarkko When use openssl to generate a key instead of using the built-in sign_key.pem, there is a probability that will encounter this problem. Here is a problematic key I encountered. The calculated q1 and q2 of this key are both 383 bytes, If the length is not processed, the hardware signature will fail. Why is reversing bytes the correct way to fix the issue? This is caused by the incorrect length of the reversed data. If the length of q2 is 383 bytes, the inversion will cause the first byte to be zero. For this, please refer to the signature tool in sgx sdk: https://github.com/intel/linux-sgx/blob/master/sdk/sign_tool/SignTool/sign_tool.cpp#L381 If it can be repaired, it may be possible to use to generate sign_key.pem key on fly instead of using the static key. Best regards, Tianjia
Re: [PATCH] selftests/sgx: fix EINIT failure dueto SGX_INVALID_SIGNATURE
On 3/2/21 1:54 PM, Andy Lutomirski wrote: On Mon, Mar 1, 2021 at 9:06 PM Tianjia Zhang wrote: On 3/1/21 5:54 PM, Jarkko Sakkinen wrote: On Mon, Mar 01, 2021 at 01:18:36PM +0800, Tianjia Zhang wrote: q2 is not always 384-byte length. Sometimes it only has 383-byte. What does determine this? In this case, the valid portion of q2 is reordered reversely for little endian order, and the remaining portion is filled with zero. I'm presuming that you want to say "In this case, q2 needs to be reversed because...". I'm lacking these details: 1. Why the length of Q2 can vary? 2. Why reversing the bytes is the correct measure to counter-measure this variation? /Jarkko When use openssl to generate a key instead of using the built-in sign_key.pem, there is a probability that will encounter this problem. Here is a problematic key I encountered. The calculated q1 and q2 of this key are both 383 bytes, If the length is not processed, the hardware signature will fail. Presumably the issue is that some keys have parameters that have enough leading 0 bits to be effectively shorter. The openssl API (and, sadly, a bunch of the ASN.1 stuff) treats these parameters as variable-size integers. I agree with your opinion. Thanks, Tianjia
Re: [PATCH] selftests/sgx: fix EINIT failure dueto SGX_INVALID_SIGNATURE
On 3/1/21 5:54 PM, Jarkko Sakkinen wrote: On Mon, Mar 01, 2021 at 01:18:36PM +0800, Tianjia Zhang wrote: q2 is not always 384-byte length. Sometimes it only has 383-byte. What does determine this? In this case, the valid portion of q2 is reordered reversely for little endian order, and the remaining portion is filled with zero. I'm presuming that you want to say "In this case, q2 needs to be reversed because...". I'm lacking these details: 1. Why the length of Q2 can vary? 2. Why reversing the bytes is the correct measure to counter-measure this variation? /Jarkko When use openssl to generate a key instead of using the built-in sign_key.pem, there is a probability that will encounter this problem. Here is a problematic key I encountered. The calculated q1 and q2 of this key are both 383 bytes, If the length is not processed, the hardware signature will fail. -BEGIN RSA PRIVATE KEY- MIIG4gIBAAKCAYEAnWxc9HyjCuLWtFVKm0xrkHimyeTEdx7LJpRzm07M/gLFxqwV bFEFL1SdK912H8S0yRKGzCTqrEa0AKaBhIzw19OgW1jIQx9+ybENnIYh4O+YGwKH ngTAw5Xfuw8iaPeLe3Pujg4h7ePI4cx6C98KM2tDHb0GeN35wM/2AxaWmuwMGosv kbNN2EN9zQVLIkaUtCJHH8UlfZ+QQVO32Mij46wO4O4783Hgr7PUmI7LCkk31vBT fzPch6LSgBy6UvtvBfJWo+t/Rk5aGm90JchY4+H1/23vwpkmKhRazBDbUeHVcX7f ytwJkmODIjbiapB6gf0AxQooIwJaqdRKddn/BB/IAkanG0m6COuvgP2Z9256U262 GvEWf+IHY2/DmoivAcc/koYHrRjNgcak8nPq9iTE4R9jPFj41+2r5k3AycCGlt75 HdYP1oZ/F0nTKp8yGOsf61DXaQLXPnPyjQunKGjBQONJb7Kj/8TOJjSuh7cdRqRP OXGZPwOEkhKU4QwtAgEDAoIBgGjy6KL9wgdB5Hg43GeIR7WlxIaYgvoUh28NomeJ 3f6sg9nIDkg2A3TjE3KTpBUtzdthrzLDRx2EeABvAQMIoI/iaueQhYIU/zEgs72u wUCfurysWmlYgIJj6ny0wZtPslJNSbQJa/PtMJaIUV0/XCJHghPTWaXpUSs1Tqy5 ubydXWcHdQvM3pAs/oiuMhbZuHgW2hUuGP5qYCuNJTswbUJytJX0J/ehQHUijbsJ 3LGGJTn1jP936FpsjFVofDdSSPgwF5a8TgxtIHNK8cuXq2gyblmo7afszujVJhib VqbYtL9UYwg/oibI+hFGxMGgDUqQlZg9E7/1QnMNRsubm7sWBO+hTA+fdwVY7+zh CtOLb7XDbHWF1+k+DDd2m4SibyBr7zsHkIO9DoDwHWvCSW+SICcfdTeCmxGPYfeZ P8QDxWj25zjS8e93/zgyMuiQY8T6AEajFU0VIZfhoHKeOYs8Vg3T30z+SwSVsTLl DDFq2PHkYg7dG14n3iFa0DXckwKBwQDOmlmLVVIVPQcDreS2sLkO/a44zzIyFwvA eItWkBWSF/1nY8Nh0dDw7Hn8QRMHoxC4pLjTxsGMLD9f5YAXZueRcjOuhnDfalpB 5M11A9QKQFB0ar/viq5Kyl6Jxv3PFdkszaRcwmxCdhjv/OL4kxfZ1gEvqeZLPLh5 fWdyNQrXBhbGrfmDQfs/d+yMmHzvJJ7rO9VXKHhqMU1QkjQFh7AjOj6PI58oEE8F eND4d+0Y5Mi4F+1jvBvshNbjcgPFjnMCgcEAww/Ztnu4Hm2iadEkvbQeuJiiQCFZ FJ7kDFwWUJfDxYTI6xyH3KrFZ0mSDAuoQH1V2X9njOfI9uY3nVrgLQmt2gyM7E5E JHAtPwF6KKg1r90CTl7Tex2kVzqWhnbchH8vJFe0XThCpQce0GGV2D1k9POTdsZN HdhXxBkxgLLWTLTHsr6kxVepr9qTtmYJ3qH9hjhKKjO/CzHXig9N25agtFQBnQHb VCTkc2tzYWUvJLIPI7XOv2nURULgfJhYyrLfAoHBAIm8O7I44WN+BK0emHnLJgn+ dCXfdswPXSr7B48KuQwP/kTtLOvhNfXy+/2At1pstdBt0I0vK7LIKj/uVWTvRQuh d8mu9epG5taYiPitOAbVivhHKp+xyYcxlFvZ/ooOkMiJGD3W8tb5ZfVTQfsMupE5 Vh/GmYd90FD+RPbOBzoEDy8epleBUipP8whlqJ9tv0d9OOTFpZwg3jW2zVkFIBd8 KbTCahq1igOl4KWlSLtDMHq6nkJ9Z/MDOez2rS5e9wKBwQCCCpEkUnq+88Gb4MMp Ir8luxbVa5C4ae1dkrmLD9fZAzCcva/ocdjvhmFdXRrVqOPmVO+zRTCkmXpo50Ae BnPmswidiYLC9XN/VlFwcCPKk1be6eJSE8Lk0bmu+ehYVMoYOng+JYHDWhSK67k6 05ijTQz52Yi+kDqCu3ZVzI7dzdp3KcMuOnEf5w0kRAaUa/5ZetwcIn9cy+UGtN6S ZGsi4qu+ATziw0L3nPeWQ3TDIV9tI98qRo2Dger9uuXcdz8CgcA1J+UJh7WX9kT4 OBIKkb1TftyT2LZyzBh2LcrueUIU3gka8IqI6X/B9lB6WTLCtuBGWZZLRAuuuWlL nEm2TuTtU0Ir7/3lnZ/Fmc5/Ams4cGfxl1oXdiXoARSLR6HdvIIBZ8GdUqISR1M1 IMMQtRIWomsRCfN0IUvgi0bTUkE5dZp8UFThZp22CahWgEq5h63pNF0K8hHdEyWb aaMCoAFhIcU4UBUDUxREyY7y1eUCWKAl0B4xEvJoxolbYyTvQB4= -END RSA PRIVATE KEY- good luck! Tianjia
[PATCH] selftests/sgx: fix EINIT failure dueto SGX_INVALID_SIGNATURE
q2 is not always 384-byte length. Sometimes it only has 383-byte. In this case, the valid portion of q2 is reordered reversely for little endian order, and the remaining portion is filled with zero. Signed-off-by: Tianjia Zhang --- tools/testing/selftests/sgx/sigstruct.c | 41 + 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c index dee7a3d6c5a5..92bbc5a15c39 100644 --- a/tools/testing/selftests/sgx/sigstruct.c +++ b/tools/testing/selftests/sgx/sigstruct.c @@ -55,10 +55,27 @@ static bool alloc_q1q2_ctx(const uint8_t *s, const uint8_t *m, return true; } +static void reverse_bytes(void *data, int length) +{ + int i = 0; + int j = length - 1; + uint8_t temp; + uint8_t *ptr = data; + + while (i < j) { + temp = ptr[i]; + ptr[i] = ptr[j]; + ptr[j] = temp; + i++; + j--; + } +} + static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1, uint8_t *q2) { struct q1q2_ctx ctx; + int len; if (!alloc_q1q2_ctx(s, m, )) { fprintf(stderr, "Not enough memory for Q1Q2 calculation\n"); @@ -89,8 +106,10 @@ static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1, goto out; } - BN_bn2bin(ctx.q1, q1); - BN_bn2bin(ctx.q2, q2); + len = BN_bn2bin(ctx.q1, q1); + reverse_bytes(q1, len); + len = BN_bn2bin(ctx.q2, q2); + reverse_bytes(q2, len); free_q1q2_ctx(); return true; @@ -152,22 +171,6 @@ static RSA *gen_sign_key(void) return key; } -static void reverse_bytes(void *data, int length) -{ - int i = 0; - int j = length - 1; - uint8_t temp; - uint8_t *ptr = data; - - while (i < j) { - temp = ptr[i]; - ptr[i] = ptr[j]; - ptr[j] = temp; - i++; - j--; - } -} - enum mrtags { MRECREATE = 0x0045544145524345, MREADD = 0x4145, @@ -367,8 +370,6 @@ bool encl_measure(struct encl *encl) /* BE -> LE */ reverse_bytes(sigstruct->signature, SGX_MODULUS_SIZE); reverse_bytes(sigstruct->modulus, SGX_MODULUS_SIZE); - reverse_bytes(sigstruct->q1, SGX_MODULUS_SIZE); - reverse_bytes(sigstruct->q2, SGX_MODULUS_SIZE); EVP_MD_CTX_destroy(ctx); RSA_free(key); -- 2.19.1.3.ge56e4f7
Re: [PATCH v5 3/3] x86/sgx: Remove redundant if conditions in sgx_encl_create
On 2/16/21 4:29 PM, Jarkko Sakkinen wrote: On Tue, Feb 16, 2021 at 11:31:33AM +0800, Tianjia Zhang wrote: In this scenario, there is no case where va_page is NULL, and the error has been checked. The if-condition statement here is redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang Acked-by: Jarkko Sakkinen --- arch/x86/kernel/cpu/sgx/ioctl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..719c21cca569 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + if (!va_page) + return -EIO; Why this check? The value of va_page may be a negative error value, it may be NULL, or it may be a valid pointer, so check this return value. Tianjia + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7 /Jarkko
[PATCH v5 2/3] x86/sgx: Allows ioctl PROVISION to execute before CREATE
In the function sgx_create_enclave(), the direct assignment operation of attributes_mask determines that the ioctl PROVISION operation must be executed after the ioctl CREATE operation, which will limit the flexibility of SGX developers. This patch takes the assignment of attributes_mask from the function sgx_create_enclave() has been moved to the function sgx_open(), this will allow users to perform ioctl PROVISION operations before ioctl CREATE, increase the flexibility of the API and reduce restrictions. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c index 8ce6d8371cfb..892e2a2a3221 100644 --- a/arch/x86/kernel/cpu/sgx/driver.c +++ b/arch/x86/kernel/cpu/sgx/driver.c @@ -36,6 +36,7 @@ static int sgx_open(struct inode *inode, struct file *file) return ret; } + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; file->private_data = encl; return 0; diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..1c6ecf9fbeff 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
[PATCH v5 0/3] Some optimizations related to sgx
This is an optimization of a set of sgx-related codes, each of which is independent of the patch. Because the second and third patches have conflicting dependencies, these patches are put together. --- v5 changes: * Remove the two patches with no actual value * Typo fix in commit message v4 changes: * Improvements suggested by review v3 changes: * split free_cnt count and spin lock optimization into two patches v2 changes: * review suggested changes Tianjia Zhang (3): selftests/x86: Use getauxval() to simplify the code in sgx x86/sgx: Allows ioctl PROVISION to execute before CREATE x86/sgx: Remove redundant if conditions in sgx_encl_create arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c| 8 tools/testing/selftests/sgx/main.c | 24 3 files changed, 9 insertions(+), 24 deletions(-) -- 2.19.1.3.ge56e4f7
[PATCH v5 3/3] x86/sgx: Remove redundant if conditions in sgx_encl_create
In this scenario, there is no case where va_page is NULL, and the error has been checked. The if-condition statement here is redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang Acked-by: Jarkko Sakkinen --- arch/x86/kernel/cpu/sgx/ioctl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..719c21cca569 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + if (!va_page) + return -EIO; + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7
[PATCH v5 1/3] selftests/x86: Use getauxval() to simplify the code in sgx
Simplify the sgx code implemntation by using library function getauxval() instead of a custom function to get the base address of vDSO. Signed-off-by: Tianjia Zhang Reviewed-by: Jarkko Sakkinen Acked-by: Shuah Khan --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 724cec700926..5167505fbb46 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)getauxval(AT_SYSINFO_EHDR); if (!addr) goto err; -- 2.19.1.3.ge56e4f7
Re: [PATCH v3 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
On 2/12/21 8:19 PM, Jarkko Sakkinen wrote: On Thu, Feb 11, 2021 at 02:04:12PM +0800, Tianjia Zhang wrote: Hi, Sorry for the late reply. On 1/28/21 1:40 AM, Jarkko Sakkinen wrote: I could bet some money that this does not bring any significant performance gain. Yes, this does not bring performance gains. This is not a change for performance, mainly to make the value of free_cnt look more accurate. On Sun, Jan 24, 2021 at 02:29:05PM +0800, Tianjia Zhang wrote: `section->free_cnt` represents the free page in sgx_epc_section, which is assigned once after initialization. In fact, just after the initialization is completed, the pages are in the `init_laundry_list` list and cannot be allocated. This needs to be recovered by EREMOVE of function sgx_sanitize_section() before it can be used as a page that can be allocated. The sgx_sanitize_section() will be called in the kernel thread ksgxd. This patch moves the initialization of `section->free_cnt` from the initialization function `sgx_setup_epc_section()` to the function `sgx_sanitize_section()`, and then accumulates the count after the Use single quotes instead of hyphens. successful execution of EREMOVE. This seems to be more reasonable, free_cnt will also truly reflect the allocatable free pages in EPC. Sined-off-by: Tianjia Zhang Reviewed-by: Sean Christopherson --- arch/x86/kernel/cpu/sgx/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 4465912174fd..e455ec7b3449 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -48,6 +48,7 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (!ret) { spin_lock(>lock); list_move(>list, >page_list); + section->free_cnt++; spin_unlock(>lock); Someone can try to allocate a page while sanitize process is in progress. I think it is better to keep critical sections in the form that when you leave from one, the global state is legit. Do you mean to move the critical section to protect the entire while loop? Of course, this is also possible, sanitize is a process only needed for initialization, and the possibility of conflict is very small. Best regards, Tianjia The big picture of this change to me, to be frank is that it's completely useless. Please start with the picture. /Jarkko I carefully considered your suggestion, and I will delete 2/5 and 3/5 in the next version. Best regards, Tianjia
Re: [PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
On 2/3/21 6:04 AM, Jarkko Sakkinen wrote: On Mon, Feb 01, 2021 at 09:26:53PM +0800, Tianjia Zhang wrote: In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is if-condition, i.e. dash missing Will do in the next patch. Thanks, Tianjia redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..719c21cca569 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + if (!va_page) + return -EIO; + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7 Acked-by: Jarkko Sakkinen /Jarkko
Re: [PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section()
On 2/3/21 6:00 AM, Jarkko Sakkinen wrote: On Mon, Feb 01, 2021 at 09:26:50PM +0800, Tianjia Zhang wrote: The spin lock of sgx_epc_section only locks the page_list. The EREMOVE operation and init_laundry_list is not necessary in the protection range of the spin lock. This patch reduces the lock range of the spin lock in the function sgx_sanitize_section() and only protects the operation of the page_list. Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang I'm not confident that this change has any practical value. /Jarkko As a process executed during initialization, this optimization effect may not be obvious. If possible, this critical area can be moved outside to protect the entire while loop. Best regards, Tianjia --- arch/x86/kernel/cpu/sgx/main.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index c519fc5f6948..4465912174fd 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (kthread_should_stop()) return; - /* needed for access to ->page_list: */ - spin_lock(>lock); - page = list_first_entry(>init_laundry_list, struct sgx_epc_page, list); ret = __eremove(sgx_get_epc_virt_addr(page)); - if (!ret) + if (!ret) { + spin_lock(>lock); list_move(>list, >page_list); - else + spin_unlock(>lock); + } else list_move_tail(>list, ); - spin_unlock(>lock); - cond_resched(); } -- 2.19.1.3.ge56e4f7
Re: [PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
On 2/3/21 5:57 AM, Jarkko Sakkinen wrote: On Mon, Feb 01, 2021 at 09:26:52PM +0800, Tianjia Zhang wrote: In the function sgx_create_enclave(), the direct assignment operation of attributes_mask determines that the ioctl PROVISION operation must be executed after the ioctl CREATE operation, which will limit the flexibility of sgx developers. Please write acronyms correctly. It's not 'sgx'. It's 'SGX'. Who are the "sgx developers" and how do they benefit from this? /Jarkko It mainly refers to application developers based on SGX technology. One of the benefits that this brings is that the PROVISION operation can be called before or after the enclave is created, compared to the previous PROVISION operation can only be executed after the enclave is created. Thanks, Tianjia
Re: [PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
On 2/3/21 5:54 AM, Jarkko Sakkinen wrote: On Mon, Feb 01, 2021 at 09:26:51PM +0800, Tianjia Zhang wrote: 'section->free_cnt' represents the free page in sgx_epc_section, which is assigned once after initialization. In fact, just after the initialization is completed, the pages are in the init_laundry_list list and cannot be allocated. This needs to be recovered by EREMOVE of function sgx_sanitize_section() before it can be used as a page that can be allocated. The sgx_sanitize_section() will be called in the kernel thread ksgxd. This patch moves the initialization of 'section->free_cnt' from the initialization function sgx_setup_epc_section() to the function sgx_sanitize_section(), and then accumulates the count after the successful execution of EREMOVE. This seems to be more reasonable, free_cnt will also truly reflect the allocatable free pages in EPC. Sined-off-by: Tianjia Zhang Reviewed-by: Sean Christopherson I just copy-paste my earlier response to save time sice you seem to save time by ignoring it and spamming with the same obviously illegit patch. https://lore.kernel.org/linux-sgx/ybglodsoax4cw...@kernel.org/ /Jarkko Sorry for the late reply, I already responded in the original email. Best regards, Tianjia
Re: [PATCH v3 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
Hi, Sorry for the late reply. On 1/28/21 1:40 AM, Jarkko Sakkinen wrote: I could bet some money that this does not bring any significant performance gain. Yes, this does not bring performance gains. This is not a change for performance, mainly to make the value of free_cnt look more accurate. On Sun, Jan 24, 2021 at 02:29:05PM +0800, Tianjia Zhang wrote: `section->free_cnt` represents the free page in sgx_epc_section, which is assigned once after initialization. In fact, just after the initialization is completed, the pages are in the `init_laundry_list` list and cannot be allocated. This needs to be recovered by EREMOVE of function sgx_sanitize_section() before it can be used as a page that can be allocated. The sgx_sanitize_section() will be called in the kernel thread ksgxd. This patch moves the initialization of `section->free_cnt` from the initialization function `sgx_setup_epc_section()` to the function `sgx_sanitize_section()`, and then accumulates the count after the Use single quotes instead of hyphens. >> successful execution of EREMOVE. This seems to be more reasonable, free_cnt will also truly reflect the allocatable free pages in EPC. Sined-off-by: Tianjia Zhang Reviewed-by: Sean Christopherson --- arch/x86/kernel/cpu/sgx/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 4465912174fd..e455ec7b3449 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -48,6 +48,7 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (!ret) { spin_lock(>lock); list_move(>list, >page_list); + section->free_cnt++; spin_unlock(>lock); Someone can try to allocate a page while sanitize process is in progress. I think it is better to keep critical sections in the form that when you leave from one, the global state is legit. Do you mean to move the critical section to protect the entire while loop? Of course, this is also possible, sanitize is a process only needed for initialization, and the possibility of conflict is very small. Best regards, Tianjia
[PATCH v4 0/5] Some optimizations related to sgx
This is an optimization of a set of sgx-related codes, each of which is independent of the patch. Because the second and third patches have conflicting dependencies, these patches are put together. --- v4 changes: * Improvements suggested by review v3 changes: * split free_cnt count and spin lock optimization into two patches v2 changes: * review suggested changes Tianjia Zhang (5): selftests/x86: Use getauxval() to simplify the code in sgx x86/sgx: Reduce the locking range in sgx_sanitize_section() x86/sgx: Optimize the free_cnt count in sgx_epc_section x86/sgx: Allows ioctl PROVISION to execute before CREATE x86/sgx: Remove redundant if conditions in sgx_encl_create arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c| 8 arch/x86/kernel/cpu/sgx/main.c | 13 + tools/testing/selftests/sgx/main.c | 24 4 files changed, 14 insertions(+), 32 deletions(-) -- 2.19.1.3.ge56e4f7
[PATCH v4 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
'section->free_cnt' represents the free page in sgx_epc_section, which is assigned once after initialization. In fact, just after the initialization is completed, the pages are in the init_laundry_list list and cannot be allocated. This needs to be recovered by EREMOVE of function sgx_sanitize_section() before it can be used as a page that can be allocated. The sgx_sanitize_section() will be called in the kernel thread ksgxd. This patch moves the initialization of 'section->free_cnt' from the initialization function sgx_setup_epc_section() to the function sgx_sanitize_section(), and then accumulates the count after the successful execution of EREMOVE. This seems to be more reasonable, free_cnt will also truly reflect the allocatable free pages in EPC. Sined-off-by: Tianjia Zhang Reviewed-by: Sean Christopherson --- arch/x86/kernel/cpu/sgx/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 4465912174fd..e455ec7b3449 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -48,6 +48,7 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (!ret) { spin_lock(>lock); list_move(>list, >page_list); + section->free_cnt++; spin_unlock(>lock); } else list_move_tail(>list, ); @@ -643,7 +644,6 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size, list_add_tail(>pages[i].list, >init_laundry_list); } - section->free_cnt = nr_pages; return true; } -- 2.19.1.3.ge56e4f7
[PATCH v4 2/5] x86/sgx: Reduce the locking range in sgx_sanitize_section()
The spin lock of sgx_epc_section only locks the page_list. The EREMOVE operation and init_laundry_list is not necessary in the protection range of the spin lock. This patch reduces the lock range of the spin lock in the function sgx_sanitize_section() and only protects the operation of the page_list. Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/main.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index c519fc5f6948..4465912174fd 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (kthread_should_stop()) return; - /* needed for access to ->page_list: */ - spin_lock(>lock); - page = list_first_entry(>init_laundry_list, struct sgx_epc_page, list); ret = __eremove(sgx_get_epc_virt_addr(page)); - if (!ret) + if (!ret) { + spin_lock(>lock); list_move(>list, >page_list); - else + spin_unlock(>lock); + } else list_move_tail(>list, ); - spin_unlock(>lock); - cond_resched(); } -- 2.19.1.3.ge56e4f7
[PATCH v4 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
In the function sgx_create_enclave(), the direct assignment operation of attributes_mask determines that the ioctl PROVISION operation must be executed after the ioctl CREATE operation, which will limit the flexibility of sgx developers. This patch takes the assignment of attributes_mask from the function sgx_create_enclave() has been moved to the function sgx_open(), this will allow users to perform ioctl PROVISION operations before ioctl CREATE, increase the flexibility of the API and reduce restrictions. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c index f2eac41bb4ff..fba0d0bfe976 100644 --- a/arch/x86/kernel/cpu/sgx/driver.c +++ b/arch/x86/kernel/cpu/sgx/driver.c @@ -36,6 +36,7 @@ static int sgx_open(struct inode *inode, struct file *file) return ret; } + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; file->private_data = encl; return 0; diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..1c6ecf9fbeff 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
[PATCH v4 1/5] selftests/x86: Use getauxval() to simplify the code in sgx
Simplify the sgx code implemntation by using library function getauxval() instead of a custom function to get the base address of vDSO. Signed-off-by: Tianjia Zhang --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 724cec700926..5167505fbb46 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)getauxval(AT_SYSINFO_EHDR); if (!addr) goto err; -- 2.19.1.3.ge56e4f7
[PATCH v4 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..719c21cca569 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,10 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + if (!va_page) + return -EIO; + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7
Re: [PATCH v3 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
On 1/30/21 9:26 PM, Jarkko Sakkinen wrote: On Sun, Jan 24, 2021 at 02:29:06PM +0800, Tianjia Zhang wrote: In the function sgx_create_enclave(), the direct assignment operation of attributes_mask determines that the ioctl PROVISION operation must be executed after the ioctl CREATE operation, which will limit the flexibility of sgx developers. This patch takes the assignment of `attributes_mask` from the function sgx_create_enclave() has been moved to the function sgx_open() to avoid this restriction. Signed-off-by: Tianjia Zhang The commit message should explicit that the API behaviour changes as the result. Please don't use hyphens in quoting. /Jarkko Will be improved in the next version. Best regards, Tianjia --- arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c index f2eac41bb4ff..fba0d0bfe976 100644 --- a/arch/x86/kernel/cpu/sgx/driver.c +++ b/arch/x86/kernel/cpu/sgx/driver.c @@ -36,6 +36,7 @@ static int sgx_open(struct inode *inode, struct file *file) return ret; } + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; file->private_data = encl; return 0; diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..1c6ecf9fbeff 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
Re: [PATCH v3 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
On 1/30/21 10:33 PM, Jarkko Sakkinen wrote: On Sun, Jan 24, 2021 at 02:29:07PM +0800, Tianjia Zhang wrote: In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..b0b829f1b761 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,11 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ This is fine. The check does not make sense. + + if (WARN_ONCE(!va_page, "non-empty VA page list before ECREATE")) + return -EIO; No need for this WARN_ONCE(). + + list_add(_page->list, >va_pages); This is fine. /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7 /Jarkko Will be improved in the next version. Thanks, Tianjia
Re: [PATCH v3 1/5] selftests/x86: Simplify the code to get vdso base address in sgx
Hi, On 1/26/21 2:12 AM, Jarkko Sakkinen wrote: What the short summary is saying now, is that this commit would make the existing code to use vDSO base address. It's already doing that. You could instead just "Use getauxval() to simplify the code". Also, I'd prefer to properly use upper and lower case letter, e.g. vDSO instead of vdso. Reply-To: In-Reply-To: <20210124062907.88229-2-tianjia.zh...@linux.alibaba.com> On Sun, Jan 24, 2021 at 02:29:03PM +0800, Tianjia Zhang wrote: This patch uses the library function `getauxval(AT_SYSINFO_EHDR)` instead of the custom function `vdso_get_base_addr` to obtain the Use either double or single quotation mark instead of hyphen. base address of vDSO, which will simplify the code implementation. Signed-off-by: Tianjia Zhang This needs to be imperative form, e.g. "Simplify the code implemntation by using getauxval() instead of a custom function." --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 724cec700926..365d01dea67b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)(uintptr_t)getauxval(AT_SYSINFO_EHDR); You could just case the result the result directly to void *. if (!addr) goto err; -- 2.19.1.3.ge56e4f7 /Jarkko Thanks for your suggestions, I will fix it in the v4 patchset. Best regards, Tianjia
Re: [PATCH v6 2/4] x509: Detect sm2 keys by their parameters OID
On 2/1/21 7:32 AM, Stefan Berger wrote: Detect whether a key is an sm2 type of key by its OID in the parameters array rather than assuming that everything under OID_id_ecPublicKey is sm2, which is not the case. Signed-off-by: Stefan Berger Cc: David Howells Cc: keyri...@vger.kernel.org --- crypto/asymmetric_keys/x509_cert_parser.c | 12 +++- include/linux/oid_registry.h | 1 + lib/oid_registry.c| 13 + 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 52c9b455fc7d..1621ceaf5c95 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -459,6 +459,7 @@ int x509_extract_key_data(void *context, size_t hdrlen, const void *value, size_t vlen) { struct x509_parse_context *ctx = context; + enum OID oid; ctx->key_algo = ctx->last_oid; switch (ctx->last_oid) { @@ -470,7 +471,16 @@ int x509_extract_key_data(void *context, size_t hdrlen, ctx->cert->pub->pkey_algo = "ecrdsa"; break; case OID_id_ecPublicKey: - ctx->cert->pub->pkey_algo = "sm2"; + if (parse_OID(ctx->params, ctx->params_size, ) != 0) + return -EBADMSG; + + switch (oid) { + case OID_sm2: + ctx->cert->pub->pkey_algo = "sm2"; + break; + default: + return -ENOPKG; + } break; default: return -ENOPKG; diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index 4462ed2c18cd..d4982e42c0d2 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -117,6 +117,7 @@ enum OID { }; extern enum OID look_up_OID(const void *data, size_t datasize); +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); extern int sprint_oid(const void *, size_t, char *, size_t); extern int sprint_OID(enum OID, char *, size_t); diff --git a/lib/oid_registry.c b/lib/oid_registry.c index f7ad43f28579..508e0b34b5f0 100644 --- a/lib/oid_registry.c +++ b/lib/oid_registry.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "oid_registry_data.c" MODULE_DESCRIPTION("OID Registry"); @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) } EXPORT_SYMBOL_GPL(look_up_OID); +int parse_OID(const void *data, size_t datasize, enum OID *oid) +{ + const unsigned char *v = data; + + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) + return -EBADMSG; + + *oid = look_up_OID(data + 2, datasize - 2); + return 0; +} +EXPORT_SYMBOL_GPL(parse_OID); + /* * sprint_OID - Print an Object Identifier into a buffer * @data: The encoded OID to print Great job, I'm just curious why we need to add a new function, this seems unnecessary, if possible, please add Reviewed-by: Tianjia Zhang Best regards, Tianjia
Re: [PATCH v6 4/4] ima: Support EC keys for signature verification
On 2/1/21 7:33 AM, Stefan Berger wrote: Add support for IMA signature verification for EC keys. Since SHA type of hashes can be used by RSA and ECDSA signature schemes we need to look at the key and derive from the key which signature scheme to use. Since this can be applied to all types of keys, we change the selection of the encoding type to be driven by the key's signature scheme rather than by the hash type. Signed-off-by: Stefan Berger Reviewed-by: Vitaly Chikunov Cc: Mimi Zohar Cc: Dmitry Kasatkin Cc: linux-integr...@vger.kernel.org Cc: Vitaly Chikunov Cc: Tianjia Zhang Cc: David Howells Cc: keyri...@vger.kernel.org --- include/keys/asymmetric-type.h | 6 ++ security/integrity/digsig_asymmetric.c | 29 -- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h index a29d3ff2e7e8..c432fdb8547f 100644 --- a/include/keys/asymmetric-type.h +++ b/include/keys/asymmetric-type.h @@ -72,6 +72,12 @@ const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key) return key->payload.data[asym_key_ids]; } +static inline +const struct public_key *asymmetric_key_public_key(const struct key *key) +{ + return key->payload.data[asym_crypto]; +} + extern struct key *find_asymmetric_key(struct key *keyring, const struct asymmetric_key_id *id_0, const struct asymmetric_key_id *id_1, diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index a662024b4c70..29002d016607 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -84,6 +84,7 @@ int asymmetric_verify(struct key *keyring, const char *sig, { struct public_key_signature pks; struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig; + const struct public_key *pk; struct key *key; int ret; @@ -105,23 +106,19 @@ int asymmetric_verify(struct key *keyring, const char *sig, memset(, 0, sizeof(pks)); pks.hash_algo = hash_algo_name[hdr->hash_algo]; - switch (hdr->hash_algo) { - case HASH_ALGO_STREEBOG_256: - case HASH_ALGO_STREEBOG_512: - /* EC-RDSA and Streebog should go together. */ - pks.pkey_algo = "ecrdsa"; - pks.encoding = "raw"; - break; - case HASH_ALGO_SM3_256: - /* SM2 and SM3 should go together. */ - pks.pkey_algo = "sm2"; - pks.encoding = "raw"; - break; - default: - pks.pkey_algo = "rsa"; + + pk = asymmetric_key_public_key(key); + pks.pkey_algo = pk->pkey_algo; + if (!strcmp(pk->pkey_algo, "rsa")) pks.encoding = "pkcs1"; - break; - } + else if (!strcmp(pk->pkey_algo, "ecdsa")) + pks.encoding = "x962"; + else if (!strcmp(pk->pkey_algo, "ecrdsa") || + !strcmp(pk->pkey_algo, "sm2")) + pks.encoding = "raw"; + else + return -ENOPKG; + pks.digest = (u8 *)data; pks.digest_size = datalen; pks.s = hdr->sig; Looks good to me, so Reviewed-by: Tianjia Zhang Thanks, Tianjia
Re: [PATCH] x86/sgx: Allows ioctl PROVISION to execute before CREATE
On 1/21/21 6:37 AM, Jarkko Sakkinen wrote: On Thu, Jan 21, 2021 at 12:34:49AM +0200, Jarkko Sakkinen wrote: On Wed, Jan 20, 2021 at 11:57:18AM +0800, Tianjia Zhang wrote: Hi, On 1/20/21 4:05 AM, Sean Christopherson wrote: On Mon, Jan 18, 2021, Tianjia Zhang wrote: In function sgx_encl_create(), the logic of directly assigning value to attributes_mask determines that the call to SGX_IOC_ENCLAVE_PROVISION must be after the command of SGX_IOC_ENCLAVE_CREATE. If change this assignment statement to or operation, the PROVISION command can be executed earlier and more flexibly. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index f45957c05f69..0ca3fc238bc2 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -108,7 +108,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; + encl->attributes_mask |= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; Alternatively, move the existing code to sgx_open()? Initializing the field when the encl object is allocated feels more correct. This seems like a good idea. Thanks for your suggestion. I have sent v2 patch, include the next two patches. Did you ask from Sean about suggested-by's? Now it looks like that doing these patches were originally proposed by Sean. Please do not add tags from people *unauthentically*. I do not see anything from Sean to any of the patches that would suggest adding those tags. You are basically just stamping that to all patches, which he has given a code review. Can you stop doing this? /Jarkko I am very sorry for the trouble caused to you, I have made improvements in the new patch, thanks for your suggestions. Best regards, Tianjia
Re: [PATCH v2] x86/sgx: Remove redundant if conditions in sgx_encl_create
On 1/21/21 1:47 AM, Sean Christopherson wrote: On Wed, Jan 20, 2021, Tianjia Zhang wrote: In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Reported-by: Jia Zhang Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..efad2fb61c76 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,12 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + + if (WARN_ONCE(!va_page, + "the tail page of the VA page list had free slots\n")) IMO it's worth trimming down the message to keep this on a single line. The newline isn't necessary, and this code expects a completely empty list, e.g. it can be reworded to something like: if (WARN_ONCE(!va_page, "non-empty VA page list before ECREATE")) Thanks for your suggestion, I have resubmitted a set of patches. Best regards, Tianjia
[PATCH v3 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
In the function sgx_create_enclave(), the direct assignment operation of attributes_mask determines that the ioctl PROVISION operation must be executed after the ioctl CREATE operation, which will limit the flexibility of sgx developers. This patch takes the assignment of `attributes_mask` from the function sgx_create_enclave() has been moved to the function sgx_open() to avoid this restriction. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c index f2eac41bb4ff..fba0d0bfe976 100644 --- a/arch/x86/kernel/cpu/sgx/driver.c +++ b/arch/x86/kernel/cpu/sgx/driver.c @@ -36,6 +36,7 @@ static int sgx_open(struct inode *inode, struct file *file) return ret; } + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; file->private_data = encl; return 0; diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..1c6ecf9fbeff 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
[PATCH v3 0/5] Some optimizations related to sgx
This is an optimization of a set of sgx-related codes, each of which is independent of the patch. Because the second and third patches have conflicting dependencies, these patches are put together. --- v3 changes: * split free_cnt count and spin lock optimization into two patches v2 changes: * review suggested changes Tianjia Zhang (5): selftests/x86: Simplify the code to get vdso base address in sgx x86/sgx: Optimize the locking range in sgx_sanitize_section() x86/sgx: Optimize the free_cnt count in sgx_epc_section x86/sgx: Allows ioctl PROVISION to execute before CREATE x86/sgx: Remove redundant if conditions in sgx_encl_create arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c| 9 + arch/x86/kernel/cpu/sgx/main.c | 13 + tools/testing/selftests/sgx/main.c | 24 4 files changed, 15 insertions(+), 32 deletions(-) -- 2.19.1.3.ge56e4f7
[PATCH v3 2/5] x86/sgx: Optimize the locking range in sgx_sanitize_section()
The spin lock of sgx_epc_section only locks the page_list. The EREMOVE operation and init_laundry_list is not necessary in the protection range of the spin lock. This patch reduces the lock range of the spin lock in the function sgx_sanitize_section() and only protects the operation of the page_list. Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/main.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index c519fc5f6948..4465912174fd 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (kthread_should_stop()) return; - /* needed for access to ->page_list: */ - spin_lock(>lock); - page = list_first_entry(>init_laundry_list, struct sgx_epc_page, list); ret = __eremove(sgx_get_epc_virt_addr(page)); - if (!ret) + if (!ret) { + spin_lock(>lock); list_move(>list, >page_list); - else + spin_unlock(>lock); + } else list_move_tail(>list, ); - spin_unlock(>lock); - cond_resched(); } -- 2.19.1.3.ge56e4f7
[PATCH v3 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
`section->free_cnt` represents the free page in sgx_epc_section, which is assigned once after initialization. In fact, just after the initialization is completed, the pages are in the `init_laundry_list` list and cannot be allocated. This needs to be recovered by EREMOVE of function sgx_sanitize_section() before it can be used as a page that can be allocated. The sgx_sanitize_section() will be called in the kernel thread ksgxd. This patch moves the initialization of `section->free_cnt` from the initialization function `sgx_setup_epc_section()` to the function `sgx_sanitize_section()`, and then accumulates the count after the successful execution of EREMOVE. This seems to be more reasonable, free_cnt will also truly reflect the allocatable free pages in EPC. Sined-off-by: Tianjia Zhang Reviewed-by: Sean Christopherson --- arch/x86/kernel/cpu/sgx/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 4465912174fd..e455ec7b3449 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -48,6 +48,7 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (!ret) { spin_lock(>lock); list_move(>list, >page_list); + section->free_cnt++; spin_unlock(>lock); } else list_move_tail(>list, ); @@ -643,7 +644,6 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size, list_add_tail(>pages[i].list, >init_laundry_list); } - section->free_cnt = nr_pages; return true; } -- 2.19.1.3.ge56e4f7
[PATCH v3 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..b0b829f1b761 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,11 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + + if (WARN_ONCE(!va_page, "non-empty VA page list before ECREATE")) + return -EIO; + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7
[PATCH v3 1/5] selftests/x86: Simplify the code to get vdso base address in sgx
This patch uses the library function `getauxval(AT_SYSINFO_EHDR)` instead of the custom function `vdso_get_base_addr` to obtain the base address of vDSO, which will simplify the code implementation. Signed-off-by: Tianjia Zhang --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 724cec700926..365d01dea67b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)(uintptr_t)getauxval(AT_SYSINFO_EHDR); if (!addr) goto err; -- 2.19.1.3.ge56e4f7
Re: [PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing
On 1/15/21 5:23 PM, Jarkko Sakkinen wrote: On Thu, Jan 14, 2021 at 12:12:16PM +0800, Tianjia Zhang wrote: On 1/14/21 10:51 AM, Jarkko Sakkinen wrote: On Wed, Jan 13, 2021 at 08:00:21PM +0800, Tianjia Zhang wrote: In tpm_tis_core_init(), tpm2_probe() will be called first, this function will eventually call tpm_tis_send(), and then tpm_tis_probe_irq_single() will detect whether the interrupt is normal, mainly the installation interrupted, set `priv->irq_tested` to false. The logic will eventually be executed to tpm_tis_send() to trigger an interrupt. There is currently such a scenario, which will cause the IRQ probe code to never be executed, so that the TPM device is in polling mode: after setting irq_tested to false, an interrupt occurs between entering the ttpm_tis_send() function, and the interrupt will be first set irq_tested to true will cause the IRQ probe code to never be executed. Can you describe the scenario more detail? The problematic scenario we encountered is like this. The following figure shows the execution flow of tpm_tis_core_init(). An interrupt occurred before the IRQ probe. This interrupt was caused by tpm2_probe(), but it was triggered before the IRQ probe was executed, and the interrupt handler would set irq_tested to true, so the IRQ probe code can never execute, that is, the code marked 2 in the figure will never happen. TPM_INT_ENABLE is cleared on reset [*]. [*] Section 5.9.1 https://trustedcomputinggroup.org/resource/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/ /Jarkko Hi, I got it, this seems to be a firmware issue. Thanks for your reply. Best regards, Tianjia
[PATCH v2] x86/sgx: Allows ioctl PROVISION to execute before CREATE
In function sgx_encl_create(), the logic of directly assigning value to attributes_mask determines that the call to SGX_IOC_ENCLAVE_PROVISION must be after the command of SGX_IOC_ENCLAVE_CREATE. If move this assignment statement to function sgx_open(), the PROVISION command can be executed earlier and more flexibly. Reported-by: Jia Zhang Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/driver.c | 3 +++ arch/x86/kernel/cpu/sgx/ioctl.c | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c index f2eac41bb4ff..8766580194ae 100644 --- a/arch/x86/kernel/cpu/sgx/driver.c +++ b/arch/x86/kernel/cpu/sgx/driver.c @@ -36,6 +36,9 @@ static int sgx_open(struct inode *inode, struct file *file) return ret; } + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | + SGX_ATTR_KSS; + file->private_data = encl; return 0; diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..1c6ecf9fbeff 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
Re: [PATCH] x86/sgx: Allows ioctl PROVISION to execute before CREATE
Hi, On 1/20/21 4:05 AM, Sean Christopherson wrote: On Mon, Jan 18, 2021, Tianjia Zhang wrote: In function sgx_encl_create(), the logic of directly assigning value to attributes_mask determines that the call to SGX_IOC_ENCLAVE_PROVISION must be after the command of SGX_IOC_ENCLAVE_CREATE. If change this assignment statement to or operation, the PROVISION command can be executed earlier and more flexibly. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index f45957c05f69..0ca3fc238bc2 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -108,7 +108,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; + encl->attributes_mask |= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; Alternatively, move the existing code to sgx_open()? Initializing the field when the encl object is allocated feels more correct. This seems like a good idea. Thanks for your suggestion. I have sent v2 patch, include the next two patches. Best regards, Tianjia
[PATCH v2] x86/sgx: Fix free_cnt counting logic in epc section
Increase `section->free_cnt` in sgx_sanitize_section() is more reasonable, which is called in ksgxd kernel thread, instead of assigning it to epc section pages number at initialization. Although this is unlikely to fail, these pages cannot be allocated after initialization, and which need to be reset by ksgxd. At the same time, taking section->lock could be moved inside the !ret flow so that EREMOVE is done without holding the lock. it's theoretically possible that ksgxd hasn't finished sanitizing the EPC when userspace starts creating enclaves. Reported-by: Jia Zhang Suggested-by: Sean Christopherson Reviewed-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/main.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index c519fc5f6948..34a72a147983 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -41,16 +41,18 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) if (kthread_should_stop()) return; - /* needed for access to ->page_list: */ - spin_lock(>lock); - page = list_first_entry(>init_laundry_list, struct sgx_epc_page, list); ret = __eremove(sgx_get_epc_virt_addr(page)); - if (!ret) + + /* needed for access to ->page_list: */ + spin_lock(>lock); + + if (!ret) { list_move(>list, >page_list); - else + section->free_cnt += 1; + } else list_move_tail(>list, ); spin_unlock(>lock); @@ -646,7 +648,6 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size, list_add_tail(>pages[i].list, >init_laundry_list); } - section->free_cnt = nr_pages; return true; } -- 2.19.1.3.ge56e4f7
[PATCH v2] x86/sgx: Remove redundant if conditions in sgx_encl_create
In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Reported-by: Jia Zhang Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..efad2fb61c76 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,12 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + + if (WARN_ONCE(!va_page, + "the tail page of the VA page list had free slots\n")) + return -EIO; + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7
[PATCH] mm: Optimizing error condition detection in do_mprotect_pkey()
Obviously, the error variable detection of the if statement is for the mprotect callback function, so it is also put into the scope of calling callbck. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- mm/mprotect.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mm/mprotect.c b/mm/mprotect.c index ab709023e9aa..94188df1ee55 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -617,10 +617,11 @@ static int do_mprotect_pkey(unsigned long start, size_t len, if (tmp > end) tmp = end; - if (vma->vm_ops && vma->vm_ops->mprotect) + if (vma->vm_ops && vma->vm_ops->mprotect) { error = vma->vm_ops->mprotect(vma, nstart, tmp, newflags); - if (error) - goto out; + if (error) + goto out; + } error = mprotect_fixup(vma, , nstart, tmp, newflags); if (error) -- 2.19.1.3.ge56e4f7
[PATCH] x86/sgx: Allows ioctl PROVISION to execute before CREATE
In function sgx_encl_create(), the logic of directly assigning value to attributes_mask determines that the call to SGX_IOC_ENCLAVE_PROVISION must be after the command of SGX_IOC_ENCLAVE_CREATE. If change this assignment statement to or operation, the PROVISION command can be executed earlier and more flexibly. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index f45957c05f69..0ca3fc238bc2 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -108,7 +108,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; + encl->attributes_mask |= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
[PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create
In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..f45957c05f69 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,8 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7
[PATCH] x86/sgx: Fix free_cnt counting logic in epc section
Increase `section->free_cnt` in sgx_sanitize_section() is more reasonable, which is called in ksgxd kernel thread, instead of assigning it to epc section pages number at initialization. Although this is unlikely to fail, these pages cannot be allocated after initialization, and which need to be reset by ksgxd. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index c519fc5f6948..9e9a3cf7c00b 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -48,9 +48,10 @@ static void sgx_sanitize_section(struct sgx_epc_section *section) struct sgx_epc_page, list); ret = __eremove(sgx_get_epc_virt_addr(page)); - if (!ret) + if (!ret) { list_move(>list, >page_list); - else + section->free_cnt += 1; + } else list_move_tail(>list, ); spin_unlock(>lock); @@ -646,7 +647,6 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size, list_add_tail(>pages[i].list, >init_laundry_list); } - section->free_cnt = nr_pages; return true; } -- 2.19.1.3.ge56e4f7
[PATCH] selftests/x86: Simplify the code of getting vdso base address in sgx
The base address of vDSO can be obtained through the library function `getauxval()`, so use `getauxval(AT_SYSINFO_EHDR)` instead of a custom implementation to simplify the code. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- tools/testing/selftests/sgx/main.c | 24 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 724cec700926..365d01dea67b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "defines.h" #include "main.h" #include "../kselftest.h" @@ -28,24 +29,6 @@ struct vdso_symtab { Elf64_Word *elf_hashtab; }; -static void *vdso_get_base_addr(char *envp[]) -{ - Elf64_auxv_t *auxv; - int i; - - for (i = 0; envp[i]; i++) - ; - - auxv = (Elf64_auxv_t *)[i + 1]; - - for (i = 0; auxv[i].a_type != AT_NULL; i++) { - if (auxv[i].a_type == AT_SYSINFO_EHDR) - return (void *)auxv[i].a_un.a_val; - } - - return NULL; -} - static Elf64_Dyn *vdso_get_dyntab(void *addr) { Elf64_Ehdr *ehdr = addr; @@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r return 0; } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sgx_enclave_run run; struct vdso_symtab symtab; @@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[]) memset(, 0, sizeof(run)); run.tcs = encl.encl_base; - addr = vdso_get_base_addr(envp); + /* Get vDSO base address */ + addr = (void *)(uintptr_t)getauxval(AT_SYSINFO_EHDR); if (!addr) goto err; -- 2.19.1.3.ge56e4f7
Re: [PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing
On 1/14/21 10:51 AM, Jarkko Sakkinen wrote: On Wed, Jan 13, 2021 at 08:00:21PM +0800, Tianjia Zhang wrote: In tpm_tis_core_init(), tpm2_probe() will be called first, this function will eventually call tpm_tis_send(), and then tpm_tis_probe_irq_single() will detect whether the interrupt is normal, mainly the installation interrupted, set `priv->irq_tested` to false. The logic will eventually be executed to tpm_tis_send() to trigger an interrupt. There is currently such a scenario, which will cause the IRQ probe code to never be executed, so that the TPM device is in polling mode: after setting irq_tested to false, an interrupt occurs between entering the ttpm_tis_send() function, and the interrupt will be first set irq_tested to true will cause the IRQ probe code to never be executed. Can you describe the scenario more detail? The problematic scenario we encountered is like this. The following figure shows the execution flow of tpm_tis_core_init(). An interrupt occurred before the IRQ probe. This interrupt was caused by tpm2_probe(), but it was triggered before the IRQ probe was executed, and the interrupt handler would set irq_tested to true, so the IRQ probe code can never execute, that is, the code marked 2 in the figure will never happen. IRQ tpm_tis_core_init() tpm2_probe() tpm_tis_send() ---+ | tpm_tis_probe_irq_single()| | devm_request_irq() | 1 priv->irq_tested = false| tpm_tis_gen_interrupt() | | tpm_tis_send()| irq_tested = true | <--+ if (priv->irq_tested) return tpm_tis_send_main() /* probe IRQ */ tpm_tis_send_main() + | 2 chip->flags |= FLAG_IRQ <---+ priv->irq_tested = true Best regards, Tianjia
[PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing
In tpm_tis_core_init(), tpm2_probe() will be called first, this function will eventually call tpm_tis_send(), and then tpm_tis_probe_irq_single() will detect whether the interrupt is normal, mainly the installation interrupted, set `priv->irq_tested` to false. The logic will eventually be executed to tpm_tis_send() to trigger an interrupt. There is currently such a scenario, which will cause the IRQ probe code to never be executed, so that the TPM device is in polling mode: after setting irq_tested to false, an interrupt occurs between entering the ttpm_tis_send() function, and the interrupt will be first set irq_tested to true will cause the IRQ probe code to never be executed. It seems that this interrupt comes from tpm2_probe(). Although the interrupt has not been installed when tpm2_probe() is called, the interrupt of tpm2_probe() is only received after IRQ detection. This patch solves this issue by introducing a new variable, which is only used in interrupts, and irq_tested only marks whether the interrupt test has been completed. Signed-off-by: Tianjia Zhang --- drivers/char/tpm/tpm_tis_core.c | 8 drivers/char/tpm/tpm_tis_core.h | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index 92c51c6cfd1b..d7589b0b3e56 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -502,7 +502,7 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) int rc, irq; struct tpm_tis_data *priv = dev_get_drvdata(>dev); - if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested) + if (priv->irq_tested) return tpm_tis_send_main(chip, buf, len); /* Verify receipt of the expected IRQ */ @@ -512,9 +512,9 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len) rc = tpm_tis_send_main(chip, buf, len); priv->irq = irq; chip->flags |= TPM_CHIP_FLAG_IRQ; - if (!priv->irq_tested) + if (!priv->int_count) tpm_msleep(1); - if (!priv->irq_tested) + if (!priv->int_count) disable_interrupts(chip); priv->irq_tested = true; return rc; @@ -725,7 +725,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id) if (interrupt == 0) return IRQ_NONE; - priv->irq_tested = true; + priv->int_count += 1; if (interrupt & TPM_INTF_DATA_AVAIL_INT) wake_up_interruptible(>read_queue); if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT) diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h index 9b2d32a59f67..c6845672f6f7 100644 --- a/drivers/char/tpm/tpm_tis_core.h +++ b/drivers/char/tpm/tpm_tis_core.h @@ -90,6 +90,7 @@ struct tpm_tis_data { int locality; int irq; bool irq_tested; + unsigned int int_count; unsigned int flags; void __iomem *ilb_base_addr; u16 clkrun_enabled; -- 2.19.1.3.ge56e4f7
Re: [PATCH] X.509: Fix crash caused by NULL pointer
On 1/7/21 6:58 PM, David Howells wrote: Tianjia Zhang wrote: On the following call path, `sig->pkey_algo` is not assigned in asymmetric_key_verify_signature(), which causes runtime crash in public_key_verify_signature(). keyctl_pkey_verify asymmetric_key_verify_signature verify_signature public_key_verify_signature This patch simply check this situation and fixes the crash caused by NULL pointer. Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") Cc: sta...@vger.kernel.org # v5.10+ Reported-by: Tobias Markus Signed-off-by: Tianjia Zhang Looks reasonable: Acked-by: David Howells I wonder, though, if cert_sig_digest_update() should be obtained by some sort of function pointer. It doesn't really seem to belong in this file. But this is a separate issue. David Yes, this is indeed the logic of the SM2 module. I have tried to dynamically load and obtain the pointer of this function through `request_module` before, but this method still does not seem very suitable. Here are some unfinished codes I tried before: https://github.com/uudiin/linux/commit/55bca48c6282415d94c53a7692622d544da99342 It would be great if you have some good experience to share with me, I will continue to try to optimize this code. Best regards, Tianjia
[PATCH] X.509: Fix crash caused by NULL pointer
On the following call path, `sig->pkey_algo` is not assigned in asymmetric_key_verify_signature(), which causes runtime crash in public_key_verify_signature(). keyctl_pkey_verify asymmetric_key_verify_signature verify_signature public_key_verify_signature This patch simply check this situation and fixes the crash caused by NULL pointer. Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") Cc: sta...@vger.kernel.org # v5.10+ Reported-by: Tobias Markus Signed-off-by: Tianjia Zhang --- crypto/asymmetric_keys/public_key.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 8892908ad58c..788a4ba1e2e7 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -356,7 +356,8 @@ int public_key_verify_signature(const struct public_key *pkey, if (ret) goto error_free_key; - if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { + if (sig->pkey_algo && strcmp(sig->pkey_algo, "sm2") == 0 && + sig->data_size) { ret = cert_sig_digest_update(sig, tfm); if (ret) goto error_free_key; -- 2.19.1.3.ge56e4f7
Re: [PATCH] crypto: ecrdsa - use subsys_initcall instead of module_init
On 11/30/20 10:24 AM, Herbert Xu wrote: On Mon, Nov 30, 2020 at 10:21:56AM +0800, Tianjia Zhang wrote: That is true only if there are non-generic implementations of the algorithms, which is not the case here. Please explain the real reason why this is needed. This is a generic algorithm, the author Vitaly Chikunov has also confirmed it, please consider this patch again. As I said, the generic algorithm only needs to be loaded early *if* there are non-generic implementations. Cheers, For ecrdsa, there is no reason to advance the initialization to subsys_init, this is just to make code clean up to have algorithm initialization uniform with other implementations. It’s just that I think that in the commit c4741b230597 ("crypto: run initcalls for generic implementations earlier"), the modification to ecrdsa happened to be omitted, because from the point of commit time, it was submitted at the same time as the ecrdsa commits, and it may happen to be omitted for ecrdsa. Best regards, Tianjia
Re: [PATCH] crypto: ecrdsa - use subsys_initcall instead of module_init
Hi Herbert, On 10/15/20 8:05 PM, Herbert Xu wrote: On Thu, Oct 15, 2020 at 07:02:41PM +0800, Tianjia Zhang wrote: All templates and generic algorithms have been registered in subsys_initcall instead of module_init. The ecrdsa algorithm happened to be missed. Here is a fix for it. That is true only if there are non-generic implementations of the algorithms, which is not the case here. Please explain the real reason why this is needed. Cheers, This is a generic algorithm, the author Vitaly Chikunov has also confirmed it, please consider this patch again. Thanks.
[PATCH] crypto: public_key: Remove redundant header file from public_key.h
The akcipher.h header file was originally introduced in SM2, and then the definition of SM2 was moved to the existing code. This header file is left and should be removed. Signed-off-by: Tianjia Zhang --- include/crypto/public_key.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h index 948c5203ca9c..47accec68cb0 100644 --- a/include/crypto/public_key.h +++ b/include/crypto/public_key.h @@ -12,7 +12,6 @@ #include #include -#include /* * Cryptographic data for the public-key subtype of the asymmetric key type. -- 2.19.1.3.ge56e4f7
[PATCH] crypto: ecrdsa - use subsys_initcall instead of module_init
All templates and generic algorithms have been registered in subsys_initcall instead of module_init. The ecrdsa algorithm happened to be missed. Here is a fix for it. Cc: Vitaly Chikunov Signed-off-by: Tianjia Zhang --- crypto/ecrdsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/ecrdsa.c b/crypto/ecrdsa.c index 6a3fd09057d0..ca9a34356f80 100644 --- a/crypto/ecrdsa.c +++ b/crypto/ecrdsa.c @@ -288,7 +288,7 @@ static void __exit ecrdsa_mod_fini(void) crypto_unregister_akcipher(_alg); } -module_init(ecrdsa_mod_init); +subsys_initcall(ecrdsa_mod_init); module_exit(ecrdsa_mod_fini); MODULE_LICENSE("GPL"); -- 2.19.1.3.ge56e4f7
[PATCH] crypto: sm2 - remove unnecessary reset operations
This is an algorithm optimization. The reset operation when setting the public key is repeated and redundant, so remove it. At the same time, `sm2_ecc_os2ec()` is optimized to make the function more simpler and more in line with the Linux code style. Signed-off-by: Tianjia Zhang --- crypto/sm2.c | 75 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/crypto/sm2.c b/crypto/sm2.c index 767e160333f6..b21addc3ac06 100644 --- a/crypto/sm2.c +++ b/crypto/sm2.c @@ -119,12 +119,6 @@ static void sm2_ec_ctx_deinit(struct mpi_ec_ctx *ec) memset(ec, 0, sizeof(*ec)); } -static int sm2_ec_ctx_reset(struct mpi_ec_ctx *ec) -{ - sm2_ec_ctx_deinit(ec); - return sm2_ec_ctx_init(ec); -} - /* RESULT must have been initialized and is set on success to the * point given by VALUE. */ @@ -132,55 +126,48 @@ static int sm2_ecc_os2ec(MPI_POINT result, MPI value) { int rc; size_t n; - const unsigned char *buf; - unsigned char *buf_memory; + unsigned char *buf; MPI x, y; - n = (mpi_get_nbits(value)+7)/8; - buf_memory = kmalloc(n, GFP_KERNEL); - rc = mpi_print(GCRYMPI_FMT_USG, buf_memory, n, , value); - if (rc) { - kfree(buf_memory); - return rc; - } - buf = buf_memory; + n = MPI_NBYTES(value); + buf = kmalloc(n, GFP_KERNEL); + if (!buf) + return -ENOMEM; - if (n < 1) { - kfree(buf_memory); - return -EINVAL; - } - if (*buf != 4) { - kfree(buf_memory); - return -EINVAL; /* No support for point compression. */ - } - if (((n-1)%2)) { - kfree(buf_memory); - return -EINVAL; - } - n = (n-1)/2; + rc = mpi_print(GCRYMPI_FMT_USG, buf, n, , value); + if (rc) + goto err_freebuf; + + rc = -EINVAL; + if (n < 1 || ((n - 1) % 2)) + goto err_freebuf; + /* No support for point compression */ + if (*buf != 0x4) + goto err_freebuf; + + rc = -ENOMEM; + n = (n - 1) / 2; x = mpi_read_raw_data(buf + 1, n); - if (!x) { - kfree(buf_memory); - return -ENOMEM; - } + if (!x) + goto err_freebuf; y = mpi_read_raw_data(buf + 1 + n, n); - kfree(buf_memory); - if (!y) { - mpi_free(x); - return -ENOMEM; - } + if (!y) + goto err_freex; mpi_normalize(x); mpi_normalize(y); - mpi_set(result->x, x); mpi_set(result->y, y); mpi_set_ui(result->z, 1); - mpi_free(x); - mpi_free(y); + rc = 0; - return 0; + mpi_free(y); +err_freex: + mpi_free(x); +err_freebuf: + kfree(buf); + return rc; } struct sm2_signature_ctx { @@ -399,10 +386,6 @@ static int sm2_set_pub_key(struct crypto_akcipher *tfm, MPI a; int rc; - rc = sm2_ec_ctx_reset(ec); - if (rc) - return rc; - ec->Q = mpi_point_new(0); if (!ec->Q) return -ENOMEM; -- 2.19.1.3.ge56e4f7
[PATCH] KVM: x86: filter guest NX capability for cpuid2
Original KVM_SET_CPUID has removed NX on non-NX hosts as it did before. but KVM_SET_CPUID2 does not. The two should be consistent. Signed-off-by: Tianjia Zhang --- arch/x86/kvm/cpuid.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 3fd6eec202d7..3e7ba2b11acb 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -257,6 +257,7 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu, goto out; } + cpuid_fix_nx_cap(vcpu); kvm_update_cpuid_runtime(vcpu); kvm_vcpu_after_set_cpuid(vcpu); out: -- 2.24.3 (Apple Git-128)
[PATCH] ipmi_si: Fix wrong return value in try_smi_init()
On an error exit path, a negative error code should be returned instead of a positive return value. Fixes: 90b2d4f15ff7 ("ipmi_si: Remove hacks for adding a dummy platform devices") Cc: Corey Minyard Signed-off-by: Tianjia Zhang --- drivers/char/ipmi/ipmi_si_intf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 77b8d551ae7f..dd559661c15b 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1963,7 +1963,7 @@ static int try_smi_init(struct smi_info *new_smi) /* Do this early so it's available for logs. */ if (!new_smi->io.dev) { pr_err("IPMI interface added with no device\n"); - rv = EIO; + rv = -EIO; goto out_err; } -- 2.24.3 (Apple Git-128)
[PATCH] X.509: fix error return value on the failed path
When memory allocation fails, an appropriate return value should be set. Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") Signed-off-by: Tianjia Zhang --- crypto/asymmetric_keys/public_key_sm2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/public_key_sm2.c b/crypto/asymmetric_keys/public_key_sm2.c index 7325cf21dbb4..66b614e1eccd 100644 --- a/crypto/asymmetric_keys/public_key_sm2.c +++ b/crypto/asymmetric_keys/public_key_sm2.c @@ -36,8 +36,10 @@ int cert_sig_digest_update(const struct public_key_signature *sig, desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); desc = kzalloc(desc_size, GFP_KERNEL); - if (!desc) + if (!desc) { + ret = -ENOMEM; goto error_free_tfm; + } desc->tfm = tfm; -- 2.24.3 (Apple Git-128)
[tip: timers/urgent] clocksource/drivers/h8300_timer8: Fix wrong return value in h8300_8timer_init()
The following commit has been merged into the timers/urgent branch of tip: Commit-ID: 400d033f5a599120089b5f0c54d14d198499af5a Gitweb: https://git.kernel.org/tip/400d033f5a599120089b5f0c54d14d198499af5a Author:Tianjia Zhang AuthorDate:Sun, 02 Aug 2020 19:15:41 +08:00 Committer: Daniel Lezcano CommitterDate: Mon, 24 Aug 2020 13:01:38 +02:00 clocksource/drivers/h8300_timer8: Fix wrong return value in h8300_8timer_init() In the init function, if the call to of_iomap() fails, the return value is ENXIO instead of -ENXIO. Change to the right negative errno. Fixes: 691f8f878290f ("clocksource/drivers/h8300_timer8: Convert init function to return error") Cc: Daniel Lezcano Signed-off-by: Tianjia Zhang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20200802111541.5429-1-tianjia.zh...@linux.alibaba.com --- drivers/clocksource/h8300_timer8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c index 1d740a8..47114c2 100644 --- a/drivers/clocksource/h8300_timer8.c +++ b/drivers/clocksource/h8300_timer8.c @@ -169,7 +169,7 @@ static int __init h8300_8timer_init(struct device_node *node) return PTR_ERR(clk); } - ret = ENXIO; + ret = -ENXIO; base = of_iomap(node, 0); if (!base) { pr_err("failed to map registers for clockevent\n");
Re: [PATCH] lib/mpi: Fix unused variable warnings
Hi Herbert, Thanks for your patch, The changes look good. I have tested this patch, so, Tested-by: Tianjia Zhang Best regards, Tianjia On 9/25/20 4:19 PM, Herbert Xu wrote: On Mon, Sep 21, 2020 at 12:20:55AM +0800, Tianjia Zhang wrote: Expand the mpi library based on libgcrypt, and the ECC algorithm of mpi based on libgcrypt requires these functions. Some other algorithms will be developed based on mpi ecc, such as SM2. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang This creates some compiler warnings. ---8<--- This patch removes a number of unused variables and marks others as unused in order to silence compiler warnings about them. Fixes: a8ea8bdd9df9 ("lib/mpi: Extend the MPI library") Signed-off-by: Herbert Xu diff --git a/lib/mpi/mpi-div.c b/lib/mpi/mpi-div.c index 21332dab97d4..45beab8b9e9e 100644 --- a/lib/mpi/mpi-div.c +++ b/lib/mpi/mpi-div.c @@ -92,7 +92,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) unsigned int normalization_steps; mpi_limb_t q_limb; mpi_ptr_t marker[5]; - unsigned int marker_nlimbs[5]; int markidx = 0; /* Ensure space is enough for quotient and remainder. @@ -152,7 +151,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) * numerator would be gradually overwritten by the quotient limbs. */ if (qp == np) { /* Copy NP object to temporary space. */ - marker_nlimbs[markidx] = nsize; np = marker[markidx++] = mpi_alloc_limb_space(nsize); MPN_COPY(np, qp, nsize); } @@ -173,7 +171,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) * the most significant word. Use temporary storage not to clobber * the original contents of the denominator. */ - marker_nlimbs[markidx] = dsize; tp = marker[markidx++] = mpi_alloc_limb_space(dsize); mpihelp_lshift(tp, dp, dsize, normalization_steps); dp = tp; @@ -195,7 +192,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) if (dp == rp || (quot && (dp == qp))) { mpi_ptr_t tp; - marker_nlimbs[markidx] = dsize; tp = marker[markidx++] = mpi_alloc_limb_space(dsize); MPN_COPY(tp, dp, dsize); dp = tp; diff --git a/lib/mpi/mpi-internal.h b/lib/mpi/mpi-internal.h index d29c4537c3a3..554002182db1 100644 --- a/lib/mpi/mpi-internal.h +++ b/lib/mpi/mpi-internal.h @@ -114,7 +114,7 @@ typedef int mpi_size_t; /* (must be a signed type) */ */ #define UDIV_QRNND_PREINV(q, r, nh, nl, d, di) \ do {\ - mpi_limb_t _ql; \ + mpi_limb_t _ql __maybe_unused; \ mpi_limb_t _q, _r; \ mpi_limb_t _xh, _xl;\ umul_ppmm(_q, _ql, (nh), (di)); \ diff --git a/lib/mpi/mpi-mul.c b/lib/mpi/mpi-mul.c index 587e6335cc12..8f5fa200f297 100644 --- a/lib/mpi/mpi-mul.c +++ b/lib/mpi/mpi-mul.c @@ -21,7 +21,6 @@ void mpi_mul(MPI w, MPI u, MPI v) int usign, vsign, sign_product; int assign_wp = 0; mpi_ptr_t tmp_limb = NULL; - unsigned int tmp_limb_nlimbs = 0; if (u->nlimbs < v->nlimbs) { /* Swap U and V. */ @@ -55,7 +54,6 @@ void mpi_mul(MPI w, MPI u, MPI v) } else { /* Make U and V not overlap with W.*/ if (wp == up) { /* W and U are identical. Allocate temporary space for U. */ - tmp_limb_nlimbs = usize; up = tmp_limb = mpi_alloc_limb_space(usize); /* Is V identical too? Keep it identical with U. */ if (wp == vp) @@ -64,7 +62,6 @@ void mpi_mul(MPI w, MPI u, MPI v) MPN_COPY(up, wp, usize); } else if (wp == vp) { /* W and V are identical. Allocate temporary space for V. */ - tmp_limb_nlimbs = vsize; vp = tmp_limb = mpi_alloc_limb_space(vsize); /* Copy to the temporary space. */ MPN_COPY(vp, wp, vsize); diff --git a/lib/mpi/mpih-div.c b/lib/mpi/mpih-div.c index 182a656a1ba0..be70ee2e42d3 100644 --- a/lib/mpi/mpih-div.c +++ b/lib/mpi/mpih-div.c @@ -31,7 +31,7 @@ mpihelp_mod_1(mpi_ptr_t dividend_ptr, mpi_size_t dividend_size, { mpi_size_t i; mpi_limb_t n1, n0, r; - mpi_limb_t dummy; + mpi_limb_t dummy __maybe_unused; /* Botch: Should this be ha
Re: [PATCH] lib/mpi: Fix unused variable warnings
Hi Herbert, Thanks for your patch, I will do a test later. By the way, did you add special compilation parameters? I compile normally without warnings in gcc 6.5 and 9.3. Best regards, Tianjia On 9/25/20 4:19 PM, Herbert Xu wrote: On Mon, Sep 21, 2020 at 12:20:55AM +0800, Tianjia Zhang wrote: Expand the mpi library based on libgcrypt, and the ECC algorithm of mpi based on libgcrypt requires these functions. Some other algorithms will be developed based on mpi ecc, such as SM2. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang This creates some compiler warnings. ---8<--- This patch removes a number of unused variables and marks others as unused in order to silence compiler warnings about them. Fixes: a8ea8bdd9df9 ("lib/mpi: Extend the MPI library") Signed-off-by: Herbert Xu diff --git a/lib/mpi/mpi-div.c b/lib/mpi/mpi-div.c index 21332dab97d4..45beab8b9e9e 100644 --- a/lib/mpi/mpi-div.c +++ b/lib/mpi/mpi-div.c @@ -92,7 +92,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) unsigned int normalization_steps; mpi_limb_t q_limb; mpi_ptr_t marker[5]; - unsigned int marker_nlimbs[5]; int markidx = 0; /* Ensure space is enough for quotient and remainder. @@ -152,7 +151,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) * numerator would be gradually overwritten by the quotient limbs. */ if (qp == np) { /* Copy NP object to temporary space. */ - marker_nlimbs[markidx] = nsize; np = marker[markidx++] = mpi_alloc_limb_space(nsize); MPN_COPY(np, qp, nsize); } @@ -173,7 +171,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) * the most significant word. Use temporary storage not to clobber * the original contents of the denominator. */ - marker_nlimbs[markidx] = dsize; tp = marker[markidx++] = mpi_alloc_limb_space(dsize); mpihelp_lshift(tp, dp, dsize, normalization_steps); dp = tp; @@ -195,7 +192,6 @@ void mpi_tdiv_qr(MPI quot, MPI rem, MPI num, MPI den) if (dp == rp || (quot && (dp == qp))) { mpi_ptr_t tp; - marker_nlimbs[markidx] = dsize; tp = marker[markidx++] = mpi_alloc_limb_space(dsize); MPN_COPY(tp, dp, dsize); dp = tp; diff --git a/lib/mpi/mpi-internal.h b/lib/mpi/mpi-internal.h index d29c4537c3a3..554002182db1 100644 --- a/lib/mpi/mpi-internal.h +++ b/lib/mpi/mpi-internal.h @@ -114,7 +114,7 @@ typedef int mpi_size_t; /* (must be a signed type) */ */ #define UDIV_QRNND_PREINV(q, r, nh, nl, d, di) \ do {\ - mpi_limb_t _ql; \ + mpi_limb_t _ql __maybe_unused; \ mpi_limb_t _q, _r; \ mpi_limb_t _xh, _xl;\ umul_ppmm(_q, _ql, (nh), (di)); \ diff --git a/lib/mpi/mpi-mul.c b/lib/mpi/mpi-mul.c index 587e6335cc12..8f5fa200f297 100644 --- a/lib/mpi/mpi-mul.c +++ b/lib/mpi/mpi-mul.c @@ -21,7 +21,6 @@ void mpi_mul(MPI w, MPI u, MPI v) int usign, vsign, sign_product; int assign_wp = 0; mpi_ptr_t tmp_limb = NULL; - unsigned int tmp_limb_nlimbs = 0; if (u->nlimbs < v->nlimbs) { /* Swap U and V. */ @@ -55,7 +54,6 @@ void mpi_mul(MPI w, MPI u, MPI v) } else { /* Make U and V not overlap with W.*/ if (wp == up) { /* W and U are identical. Allocate temporary space for U. */ - tmp_limb_nlimbs = usize; up = tmp_limb = mpi_alloc_limb_space(usize); /* Is V identical too? Keep it identical with U. */ if (wp == vp) @@ -64,7 +62,6 @@ void mpi_mul(MPI w, MPI u, MPI v) MPN_COPY(up, wp, usize); } else if (wp == vp) { /* W and V are identical. Allocate temporary space for V. */ - tmp_limb_nlimbs = vsize; vp = tmp_limb = mpi_alloc_limb_space(vsize); /* Copy to the temporary space. */ MPN_COPY(vp, wp, vsize); diff --git a/lib/mpi/mpih-div.c b/lib/mpi/mpih-div.c index 182a656a1ba0..be70ee2e42d3 100644 --- a/lib/mpi/mpih-div.c +++ b/lib/mpi/mpih-div.c @@ -31,7 +31,7 @@ mpihelp_mod_1(mpi_ptr_t dividend_ptr, mpi_size_t dividend_size, { mpi_size_t i; mpi_limb_t n1, n0, r; - mpi_limb_t dummy; + mpi_limb_t dummy __maybe_
Re: [PATCH] mm/shmem.c: Fix the missing unaccount on the failed path
On 9/21/20 2:49 AM, Hugh Dickins wrote: On Mon, 21 Sep 2020, Tianjia Zhang wrote: In function __shmem_file_setup(), shmem_unacct_size() is forgotten on the failed path, so add it. Fixes: 93dec2da7b234 ("... and switch shmem_file_setup() to alloc_file_pseudo()") Cc: Al Viro Signed-off-by: Tianjia Zhang --- mm/shmem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mm/shmem.c b/mm/shmem.c index 8e2b35ba93ad..591410dc3541 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -4200,8 +4200,10 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l if (!IS_ERR(res)) res = alloc_file_pseudo(inode, mnt, name, O_RDWR, _file_operations); - if (IS_ERR(res)) + if (IS_ERR(res)) { iput(inode); + shmem_unacct_size(flags, size); + } return res; } -- 2.19.1.3.ge56e4f7 Looks mistaken to me. Is this something you noticed by source inspection, or something you have observed in practice? I haven't tried exercising this path while injecting errors into alloc_file_pseudo(); but what I'd expect to happen is that the iput(inode), which you see already on that error path, will get to evict the inode, which will entail calling shmem_evict_inode(), which does that shmem_unacct_size() itself. Hugh I noticed by looking at the code. you are right, I neglected this point, thanks for your explanation. Thanks, Tianjia
[PATCH] mm/shmem.c: Fix the missing unaccount on the failed path
In function __shmem_file_setup(), shmem_unacct_size() is forgotten on the failed path, so add it. Fixes: 93dec2da7b234 ("... and switch shmem_file_setup() to alloc_file_pseudo()") Cc: Al Viro Signed-off-by: Tianjia Zhang --- mm/shmem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mm/shmem.c b/mm/shmem.c index 8e2b35ba93ad..591410dc3541 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -4200,8 +4200,10 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l if (!IS_ERR(res)) res = alloc_file_pseudo(inode, mnt, name, O_RDWR, _file_operations); - if (IS_ERR(res)) + if (IS_ERR(res)) { iput(inode); + shmem_unacct_size(flags, size); + } return res; } -- 2.19.1.3.ge56e4f7
[PATCH v7 06/10] crypto: testmgr - Fix potential memory leak in test_akcipher_one()
When the 'key' allocation fails, the 'req' will not be released, which will cause memory leakage on this path. This patch adds a 'free_req' tag used to solve this problem, and two new err values are added to reflect the real reason of the error. Signed-off-by: Tianjia Zhang --- crypto/testmgr.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index cd002a030af5..ed8e29efe280 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -3954,7 +3954,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, key = kmalloc(vecs->key_len + sizeof(u32) * 2 + vecs->param_len, GFP_KERNEL); if (!key) - goto free_xbuf; + goto free_req; memcpy(key, vecs->key, vecs->key_len); ptr = key + vecs->key_len; ptr = test_pack_u32(ptr, vecs->algo); @@ -3966,7 +3966,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, else err = crypto_akcipher_set_priv_key(tfm, key, vecs->key_len); if (err) - goto free_req; + goto free_key; /* * First run test which do not require a private key, such as @@ -3976,7 +3976,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, out_len_max = crypto_akcipher_maxsize(tfm); outbuf_enc = kzalloc(out_len_max, GFP_KERNEL); if (!outbuf_enc) - goto free_req; + goto free_key; if (!vecs->siggen_sigver_test) { m = vecs->m; @@ -3995,6 +3995,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, op = "verify"; } + err = -E2BIG; if (WARN_ON(m_size > PAGE_SIZE)) goto free_all; memcpy(xbuf[0], m, m_size); @@ -4061,6 +4062,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, c_size = req->dst_len; } + err = -E2BIG; op = vecs->siggen_sigver_test ? "sign" : "decrypt"; if (WARN_ON(c_size > PAGE_SIZE)) goto free_all; @@ -4097,9 +4099,10 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, free_all: kfree(outbuf_dec); kfree(outbuf_enc); +free_key: + kfree(key); free_req: akcipher_request_free(req); - kfree(key); free_xbuf: testmgr_free_buf(xbuf); return err; -- 2.19.1.3.ge56e4f7
[PATCH v7 09/10] X.509: support OSCCA SM2-with-SM3 certificate verification
The digital certificate format based on SM2 crypto algorithm as specified in GM/T 0015-2012. It was published by State Encryption Management Bureau, China. The method of generating Other User Information is defined as ZA=H256(ENTLA || IDA || a || b || xG || yG || xA || yA), it also specified in https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02. The x509 certificate supports SM2-with-SM3 type certificate verification. Because certificate verification requires ZA in addition to tbs data, ZA also depends on elliptic curve parameters and public key data, so you need to access tbs in sig and calculate ZA. Finally calculate the digest of the signature and complete the verification work. The calculation process of ZA is declared in specifications GM/T 0009-2012 and GM/T 0003.2-2012. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang Reviewed-by: Gilad Ben-Yossef --- crypto/asymmetric_keys/Makefile | 1 + crypto/asymmetric_keys/public_key.c | 6 +++ crypto/asymmetric_keys/public_key_sm2.c | 61 crypto/asymmetric_keys/x509_public_key.c | 3 ++ include/crypto/public_key.h | 15 ++ 5 files changed, 86 insertions(+) create mode 100644 crypto/asymmetric_keys/public_key_sm2.c diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile index 28b91adba2ae..1a99ea5acb6b 100644 --- a/crypto/asymmetric_keys/Makefile +++ b/crypto/asymmetric_keys/Makefile @@ -11,6 +11,7 @@ asymmetric_keys-y := \ signature.o obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key_sm2.o obj-$(CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE) += asym_tpm.o # diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index d8410ffd7f12..1d0492098bbd 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -299,6 +299,12 @@ int public_key_verify_signature(const struct public_key *pkey, if (ret) goto error_free_key; + if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { + ret = cert_sig_digest_update(sig, tfm); + if (ret) + goto error_free_key; + } + sg_init_table(src_sg, 2); sg_set_buf(_sg[0], sig->s, sig->s_size); sg_set_buf(_sg[1], sig->digest, sig->digest_size); diff --git a/crypto/asymmetric_keys/public_key_sm2.c b/crypto/asymmetric_keys/public_key_sm2.c new file mode 100644 index ..7325cf21dbb4 --- /dev/null +++ b/crypto/asymmetric_keys/public_key_sm2.c @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * asymmetric public-key algorithm for SM2-with-SM3 certificate + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + * + * Copyright (c) 2020, Alibaba Group. + * Authors: Tianjia Zhang + */ + +#include +#include +#include + +#if IS_REACHABLE(CONFIG_CRYPTO_SM2) + +int cert_sig_digest_update(const struct public_key_signature *sig, + struct crypto_akcipher *tfm_pkey) +{ + struct crypto_shash *tfm; + struct shash_desc *desc; + size_t desc_size; + unsigned char dgst[SM3_DIGEST_SIZE]; + int ret; + + BUG_ON(!sig->data); + + ret = sm2_compute_z_digest(tfm_pkey, SM2_DEFAULT_USERID, + SM2_DEFAULT_USERID_LEN, dgst); + if (ret) + return ret; + + tfm = crypto_alloc_shash(sig->hash_algo, 0, 0); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); + + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); + desc = kzalloc(desc_size, GFP_KERNEL); + if (!desc) + goto error_free_tfm; + + desc->tfm = tfm; + + ret = crypto_shash_init(desc); + if (ret < 0) + goto error_free_desc; + + ret = crypto_shash_update(desc, dgst, SM3_DIGEST_SIZE); + if (ret < 0) + goto error_free_desc; + + ret = crypto_shash_finup(desc, sig->data, sig->data_size, sig->digest); + +error_free_desc: + kfree(desc); +error_free_tfm: + crypto_free_shash(tfm); + return ret; +} + +#endif /* ! IS_REACHABLE(CONFIG_CRYPTO_SM2) */ diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index d964cc82b69c..ae450eb8be14 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -30,6 +30,9 @@ int x509_get_sig_params(struct x509_certificate *cert) pr_devel("==>%s()\n", __func__); + sig->data = cert->tbs; + sig->data_size = cert->tbs_size; + if (!cert->pub->pkey_algo) cert->unsupported_key = true; diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
[PATCH v7 05/10] crypto: testmgr - support test with different ciphertext per encryption
Some asymmetric algorithms will get different ciphertext after each encryption, such as SM2, and let testmgr support the testing of such algorithms. In struct akcipher_testvec, set c and c_size to be empty, skip the comparison of the ciphertext, and compare the decrypted plaintext with m to achieve the test purpose. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/testmgr.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 23c27fc96394..cd002a030af5 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -4025,7 +4025,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, pr_err("alg: akcipher: %s test failed. err %d\n", op, err); goto free_all; } - if (!vecs->siggen_sigver_test) { + if (!vecs->siggen_sigver_test && c) { if (req->dst_len != c_size) { pr_err("alg: akcipher: %s test failed. Invalid output len\n", op); @@ -4056,6 +4056,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, goto free_all; } + if (!vecs->siggen_sigver_test && !c) { + c = outbuf_enc; + c_size = req->dst_len; + } + op = vecs->siggen_sigver_test ? "sign" : "decrypt"; if (WARN_ON(c_size > PAGE_SIZE)) goto free_all; -- 2.19.1.3.ge56e4f7
[PATCH v7 04/10] crypto: sm2 - introduce OSCCA SM2 asymmetric cipher algorithm
This new module implement the SM2 public key algorithm. It was published by State Encryption Management Bureau, China. List of specifications for SM2 elliptic curve public key cryptography: * GM/T 0003.1-2012 * GM/T 0003.2-2012 * GM/T 0003.3-2012 * GM/T 0003.4-2012 * GM/T 0003.5-2012 IETF: https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 oscca: http://www.oscca.gov.cn/sca/xxgk/2010-12/17/content_1002386.shtml scctc: http://www.gmbz.org.cn/main/bzlb.html Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/Kconfig | 17 ++ crypto/Makefile | 8 + crypto/sm2.c | 481 +++ crypto/sm2signature.asn1 | 4 + include/crypto/sm2.h | 25 ++ 5 files changed, 535 insertions(+) create mode 100644 crypto/sm2.c create mode 100644 crypto/sm2signature.asn1 create mode 100644 include/crypto/sm2.h diff --git a/crypto/Kconfig b/crypto/Kconfig index 1b57419fa2e7..fd1b2693aca2 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -260,6 +260,23 @@ config CRYPTO_ECRDSA standard algorithms (called GOST algorithms). Only signature verification is implemented. +config CRYPTO_SM2 + tristate "SM2 algorithm" + select CRYPTO_SM3 + select CRYPTO_AKCIPHER + select CRYPTO_MANAGER + select MPILIB + select ASN1 + help + Generic implementation of the SM2 public key algorithm. It was + published by State Encryption Management Bureau, China. + as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012. + + References: + https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + http://www.oscca.gov.cn/sca/xxgk/2010-12/17/content_1002386.shtml + http://www.gmbz.org.cn/main/bzlb.html + config CRYPTO_CURVE25519 tristate "Curve25519 algorithm" select CRYPTO_KPP diff --git a/crypto/Makefile b/crypto/Makefile index 4ca12b6044f7..b279483fba50 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -42,6 +42,14 @@ rsa_generic-y += rsa_helper.o rsa_generic-y += rsa-pkcs1pad.o obj-$(CONFIG_CRYPTO_RSA) += rsa_generic.o +$(obj)/sm2signature.asn1.o: $(obj)/sm2signature.asn1.c $(obj)/sm2signature.asn1.h +$(obj)/sm2.o: $(obj)/sm2signature.asn1.h + +sm2_generic-y += sm2signature.asn1.o +sm2_generic-y += sm2.o + +obj-$(CONFIG_CRYPTO_SM2) += sm2_generic.o + crypto_acompress-y := acompress.o crypto_acompress-y += scompress.o obj-$(CONFIG_CRYPTO_ACOMP2) += crypto_acompress.o diff --git a/crypto/sm2.c b/crypto/sm2.c new file mode 100644 index ..767e160333f6 --- /dev/null +++ b/crypto/sm2.c @@ -0,0 +1,481 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * SM2 asymmetric public-key algorithm + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + * + * Copyright (c) 2020, Alibaba Group. + * Authors: Tianjia Zhang + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include "sm2signature.asn1.h" + +#define MPI_NBYTES(m) ((mpi_get_nbits(m) + 7) / 8) + +struct ecc_domain_parms { + const char *desc; /* Description of the curve. */ + unsigned int nbits; /* Number of bits. */ + unsigned int fips:1; /* True if this is a FIPS140-2 approved curve */ + + /* The model describing this curve. This is mainly used to select +* the group equation. +*/ + enum gcry_mpi_ec_models model; + + /* The actual ECC dialect used. This is used for curve specific +* optimizations and to select encodings etc. +*/ + enum ecc_dialects dialect; + + const char *p; /* The prime defining the field. */ + const char *a, *b; /* The coefficients. For Twisted Edwards +* Curves b is used for d. For Montgomery +* Curves (a,b) has ((A-2)/4,B^-1). +*/ + const char *n; /* The order of the base point. */ + const char *g_x, *g_y; /* Base point. */ + unsigned int h; /* Cofactor. */ +}; + +static const struct ecc_domain_parms sm2_ecp = { + .desc = "sm2p256v1", + .nbits = 256, + .fips = 0, + .model = MPI_EC_WEIERSTRASS, + .dialect = ECC_DIALECT_STANDARD, + .p = "0xfffe", + .a = "0xfffefffc", + .b = "0x28e9fa9e9d9f5e344d5a9e4bcf6509a7f39789f515ab8f92ddbcbd414d940e93", + .n = "0xfffe7203df6b21c6052b53bbf40939d54123", + .g_x = "0x32c4ae2c1f1981195f9904466a39c9948fe30bbff2660be1715a4589334c74c7", + .g_y = "0xbc3736a2f4f6779c59bdcee36b692153d0a987
[PATCH v7 02/10] lib/mpi: Extend the MPI library
Expand the mpi library based on libgcrypt, and the ECC algorithm of mpi based on libgcrypt requires these functions. Some other algorithms will be developed based on mpi ecc, such as SM2. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- include/linux/mpi.h| 87 +++ lib/mpi/Makefile | 5 + lib/mpi/mpi-add.c | 155 +++ lib/mpi/mpi-bit.c | 251 ++ lib/mpi/mpi-cmp.c | 46 -- lib/mpi/mpi-div.c | 238 + lib/mpi/mpi-internal.h | 53 +++ lib/mpi/mpi-inv.c | 143 ++ lib/mpi/mpi-mod.c | 155 +++ lib/mpi/mpi-mul.c | 94 lib/mpi/mpicoder.c | 336 + lib/mpi/mpih-div.c | 294 lib/mpi/mpih-mul.c | 25 +++ lib/mpi/mpiutil.c | 204 + 14 files changed, 2076 insertions(+), 10 deletions(-) create mode 100644 lib/mpi/mpi-add.c create mode 100644 lib/mpi/mpi-div.c create mode 100644 lib/mpi/mpi-inv.c create mode 100644 lib/mpi/mpi-mod.c create mode 100644 lib/mpi/mpi-mul.c diff --git a/include/linux/mpi.h b/include/linux/mpi.h index 5d906dfbf3ed..3c9e41603cf6 100644 --- a/include/linux/mpi.h +++ b/include/linux/mpi.h @@ -40,21 +40,79 @@ struct gcry_mpi { typedef struct gcry_mpi *MPI; #define mpi_get_nlimbs(a) ((a)->nlimbs) +#define mpi_has_sign(a) ((a)->sign) /*-- mpiutil.c --*/ MPI mpi_alloc(unsigned nlimbs); +void mpi_clear(MPI a); void mpi_free(MPI a); int mpi_resize(MPI a, unsigned nlimbs); +static inline MPI mpi_new(unsigned int nbits) +{ + return mpi_alloc((nbits + BITS_PER_MPI_LIMB - 1) / BITS_PER_MPI_LIMB); +} + +MPI mpi_copy(MPI a); +MPI mpi_alloc_like(MPI a); +void mpi_snatch(MPI w, MPI u); +MPI mpi_set(MPI w, MPI u); +MPI mpi_set_ui(MPI w, unsigned long u); +MPI mpi_alloc_set_ui(unsigned long u); +void mpi_swap_cond(MPI a, MPI b, unsigned long swap); + +/* Constants used to return constant MPIs. See mpi_init if you + * want to add more constants. + */ +#define MPI_NUMBER_OF_CONSTANTS 6 +enum gcry_mpi_constants { + MPI_C_ZERO, + MPI_C_ONE, + MPI_C_TWO, + MPI_C_THREE, + MPI_C_FOUR, + MPI_C_EIGHT +}; + +MPI mpi_const(enum gcry_mpi_constants no); + /*-- mpicoder.c --*/ + +/* Different formats of external big integer representation. */ +enum gcry_mpi_format { + GCRYMPI_FMT_NONE = 0, + GCRYMPI_FMT_STD = 1,/* Twos complement stored without length. */ + GCRYMPI_FMT_PGP = 2,/* As used by OpenPGP (unsigned only). */ + GCRYMPI_FMT_SSH = 3,/* As used by SSH (like STD but with length). */ + GCRYMPI_FMT_HEX = 4,/* Hex format. */ + GCRYMPI_FMT_USG = 5,/* Like STD but unsigned. */ + GCRYMPI_FMT_OPAQUE = 8 /* Opaque format (some functions only). */ +}; + MPI mpi_read_raw_data(const void *xbuffer, size_t nbytes); MPI mpi_read_from_buffer(const void *buffer, unsigned *ret_nread); +int mpi_fromstr(MPI val, const char *str); +MPI mpi_scanval(const char *string); MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int len); void *mpi_get_buffer(MPI a, unsigned *nbytes, int *sign); int mpi_read_buffer(MPI a, uint8_t *buf, unsigned buf_len, unsigned *nbytes, int *sign); int mpi_write_to_sgl(MPI a, struct scatterlist *sg, unsigned nbytes, int *sign); +int mpi_print(enum gcry_mpi_format format, unsigned char *buffer, + size_t buflen, size_t *nwritten, MPI a); + +/*-- mpi-mod.c --*/ +void mpi_mod(MPI rem, MPI dividend, MPI divisor); + +/* Context used with Barrett reduction. */ +struct barrett_ctx_s; +typedef struct barrett_ctx_s *mpi_barrett_t; + +mpi_barrett_t mpi_barrett_init(MPI m, int copy); +void mpi_barrett_free(mpi_barrett_t ctx); +void mpi_mod_barrett(MPI r, MPI x, mpi_barrett_t ctx); +void mpi_mul_barrett(MPI w, MPI u, MPI v, mpi_barrett_t ctx); /*-- mpi-pow.c --*/ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod); @@ -62,6 +120,7 @@ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod); /*-- mpi-cmp.c --*/ int mpi_cmp_ui(MPI u, ulong v); int mpi_cmp(MPI u, MPI v); +int mpi_cmpabs(MPI u, MPI v); /*-- mpi-sub-ui.c --*/ int mpi_sub_ui(MPI w, MPI u, unsigned long vval); @@ -69,6 +128,34 @@ 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); +int mpi_test_bit(MPI a, unsigned int n); +void mpi_set_bit(MPI a, unsigned int n); +void mpi_set_highbit(MPI a, unsigned int n); +void mpi_clear_highbit(MPI a, unsigned int n); +void mpi_clear_bit(MPI a, unsigned int n); +void mpi_rshift_limbs(MPI a, unsigned int count); +void mpi_rshift(MPI x, MPI a, unsigned int n); +void mpi_lshift_limbs(MPI a, unsigned int count); +void mpi_lshift(MPI x, MPI a, unsigned int n); + +/*-- mpi-add.c --*/ +void mpi_add_ui(MPI w, MPI u, unsigned long v);
[PATCH v7 07/10] crypto: sm2 - add SM2 test vectors to testmgr
Add testmgr test vectors for SM2 algorithm. These vectors come from `openssl pkeyutl -sign` and libgcrypt. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/testmgr.c | 6 + crypto/testmgr.h | 59 2 files changed, 65 insertions(+) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index ed8e29efe280..1317a63172d0 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -5384,6 +5384,12 @@ static const struct alg_test_desc alg_test_descs[] = { .suite = { .hash = __VECS(sha512_tv_template) } + }, { + .alg = "sm2", + .test = alg_test_akcipher, + .suite = { + .akcipher = __VECS(sm2_tv_template) + } }, { .alg = "sm3", .test = alg_test_hash, diff --git a/crypto/testmgr.h b/crypto/testmgr.h index b9a2d73d9f8d..8c83811c0e35 100644 --- a/crypto/testmgr.h +++ b/crypto/testmgr.h @@ -3792,6 +3792,65 @@ static const struct hash_testvec hmac_streebog512_tv_template[] = { }, }; +/* + * SM2 test vectors. + */ +static const struct akcipher_testvec sm2_tv_template[] = { + { /* Generated from openssl */ + .key = + "\x04" + "\x8e\xa0\x33\x69\x91\x7e\x3d\xec\xad\x8e\xf0\x45\x5e\x13\x3e\x68" + "\x5b\x8c\xab\x5c\xc6\xc8\x50\xdf\x91\x00\xe0\x24\x73\x4d\x31\xf2" + "\x2e\xc0\xd5\x6b\xee\xda\x98\x93\xec\xd8\x36\xaa\xb9\xcf\x63\x82" + "\xef\xa7\x1a\x03\xed\x16\xba\x74\xb8\x8b\xf9\xe5\x70\x39\xa4\x70", + .key_len = 65, + .param_len = 0, + .c = + "\x30\x45" + "\x02\x20" + "\x70\xab\xb6\x7d\xd6\x54\x80\x64\x42\x7e\x2d\x05\x08\x36\xc9\x96" + "\x25\xc2\xbb\xff\x08\xe5\x43\x15\x5e\xf3\x06\xd9\x2b\x2f\x0a\x9f" + "\x02\x21" + "\x00" + "\xbf\x21\x5f\x7e\x5d\x3f\x1a\x4d\x8f\x84\xc2\xe9\xa6\x4c\xa4\x18" + "\xb2\xb8\x46\xf4\x32\x96\xfa\x57\xc6\x29\xd4\x89\xae\xcc\xda\xdb", + .c_size = 71, + .algo = OID_SM2_with_SM3, + .m = + "\x47\xa7\xbf\xd3\xda\xc4\x79\xee\xda\x8b\x4f\xe8\x40\x94\xd4\x32" + "\x8f\xf1\xcd\x68\x4d\xbd\x9b\x1d\xe0\xd8\x9a\x5d\xad\x85\x47\x5c", + .m_size = 32, + .public_key_vec = true, + .siggen_sigver_test = true, + }, + { /* From libgcrypt */ + .key = + "\x04" + "\x87\x59\x38\x9a\x34\xaa\xad\x07\xec\xf4\xe0\xc8\xc2\x65\x0a\x44" + "\x59\xc8\xd9\x26\xee\x23\x78\x32\x4e\x02\x61\xc5\x25\x38\xcb\x47" + "\x75\x28\x10\x6b\x1e\x0b\x7c\x8d\xd5\xff\x29\xa9\xc8\x6a\x89\x06" + "\x56\x56\xeb\x33\x15\x4b\xc0\x55\x60\x91\xef\x8a\xc9\xd1\x7d\x78", + .key_len = 65, + .param_len = 0, + .c = + "\x30\x44" + "\x02\x20" + "\xd9\xec\xef\xe8\x5f\xee\x3c\x59\x57\x8e\x5b\xab\xb3\x02\xe1\x42" + "\x4b\x67\x2c\x0b\x26\xb6\x51\x2c\x3e\xfc\xc6\x49\xec\xfe\x89\xe5" + "\x02\x20" + "\x43\x45\xd0\xa5\xff\xe5\x13\x27\x26\xd0\xec\x37\xad\x24\x1e\x9a" + "\x71\x9a\xa4\x89\xb0\x7e\x0f\xc4\xbb\x2d\x50\xd0\xe5\x7f\x7a\x68", + .c_size = 70, + .algo = OID_SM2_with_SM3, + .m = + "\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\xbb\xcc\xdd\xee\xff\x00" + "\x12\x34\x56\x78\x9a\xbc\xde\xf0\x12\x34\x56\x78\x9a\xbc\xde\xf0", + .m_size = 32, + .public_key_vec = true, + .siggen_sigver_test = true, + }, +}; + /* Example vectors below taken from * http://www.oscca.gov.cn/UpFile/20101222141857786.pdf * -- 2.19.1.3.ge56e4f7
[PATCH v7 08/10] X.509: support OSCCA certificate parse
The digital certificate format based on SM2 crypto algorithm as specified in GM/T 0015-2012. It was published by State Encryption Management Bureau, China. This patch adds the OID object identifier defined by OSCCA. The x509 certificate supports SM2-with-SM3 type certificate parsing. It uses the standard elliptic curve public key, and the sm2 algorithm signs the hash generated by sm3. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang Reviewed-by: Vitaly Chikunov --- crypto/asymmetric_keys/x509_cert_parser.c | 27 ++- include/linux/oid_registry.h | 6 + 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 26ec20ef4899..52c9b455fc7d 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -234,6 +234,10 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, case OID_gost2012Signature512: ctx->cert->sig->hash_algo = "streebog512"; goto ecrdsa; + + case OID_SM2_with_SM3: + ctx->cert->sig->hash_algo = "sm3"; + goto sm2; } rsa_pkcs1: @@ -246,6 +250,11 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, ctx->cert->sig->encoding = "raw"; ctx->algo_oid = ctx->last_oid; return 0; +sm2: + ctx->cert->sig->pkey_algo = "sm2"; + ctx->cert->sig->encoding = "raw"; + ctx->algo_oid = ctx->last_oid; + return 0; } /* @@ -266,7 +275,8 @@ int x509_note_signature(void *context, size_t hdrlen, } if (strcmp(ctx->cert->sig->pkey_algo, "rsa") == 0 || - strcmp(ctx->cert->sig->pkey_algo, "ecrdsa") == 0) { + strcmp(ctx->cert->sig->pkey_algo, "ecrdsa") == 0 || + strcmp(ctx->cert->sig->pkey_algo, "sm2") == 0) { /* Discard the BIT STRING metadata */ if (vlen < 1 || *(const u8 *)value != 0) return -EBADMSG; @@ -451,13 +461,20 @@ int x509_extract_key_data(void *context, size_t hdrlen, struct x509_parse_context *ctx = context; ctx->key_algo = ctx->last_oid; - if (ctx->last_oid == OID_rsaEncryption) + switch (ctx->last_oid) { + case OID_rsaEncryption: ctx->cert->pub->pkey_algo = "rsa"; - else if (ctx->last_oid == OID_gost2012PKey256 || -ctx->last_oid == OID_gost2012PKey512) + break; + case OID_gost2012PKey256: + case OID_gost2012PKey512: ctx->cert->pub->pkey_algo = "ecrdsa"; - else + break; + case OID_id_ecPublicKey: + ctx->cert->pub->pkey_algo = "sm2"; + break; + default: return -ENOPKG; + } /* Discard the BIT STRING metadata */ if (vlen < 1 || *(const u8 *)value != 0) diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index 657d6bf2c064..4462ed2c18cd 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -107,6 +107,12 @@ enum OID { OID_gostTC26Sign512B, /* 1.2.643.7.1.2.1.2.2 */ OID_gostTC26Sign512C, /* 1.2.643.7.1.2.1.2.3 */ + /* OSCCA */ + OID_sm2,/* 1.2.156.10197.1.301 */ + OID_sm3,/* 1.2.156.10197.1.401 */ + OID_SM2_with_SM3, /* 1.2.156.10197.1.501 */ + OID_sm3WithRSAEncryption, /* 1.2.156.10197.1.504 */ + OID__NR }; -- 2.19.1.3.ge56e4f7
[PATCH v7 01/10] crypto: sm3 - export crypto_sm3_final function
Both crypto_sm3_update and crypto_sm3_finup have been exported, exporting crypto_sm3_final, to avoid having to use crypto_sm3_finup(desc, NULL, 0, dgst) to calculate the hash in some cases. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/sm3_generic.c | 7 --- include/crypto/sm3.h | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crypto/sm3_generic.c b/crypto/sm3_generic.c index 3468975215ca..193c4584bd00 100644 --- a/crypto/sm3_generic.c +++ b/crypto/sm3_generic.c @@ -149,17 +149,18 @@ int crypto_sm3_update(struct shash_desc *desc, const u8 *data, } EXPORT_SYMBOL(crypto_sm3_update); -static int sm3_final(struct shash_desc *desc, u8 *out) +int crypto_sm3_final(struct shash_desc *desc, u8 *out) { sm3_base_do_finalize(desc, sm3_generic_block_fn); return sm3_base_finish(desc, out); } +EXPORT_SYMBOL(crypto_sm3_final); int crypto_sm3_finup(struct shash_desc *desc, const u8 *data, unsigned int len, u8 *hash) { sm3_base_do_update(desc, data, len, sm3_generic_block_fn); - return sm3_final(desc, hash); + return crypto_sm3_final(desc, hash); } EXPORT_SYMBOL(crypto_sm3_finup); @@ -167,7 +168,7 @@ static struct shash_alg sm3_alg = { .digestsize = SM3_DIGEST_SIZE, .init = sm3_base_init, .update = crypto_sm3_update, - .final = sm3_final, + .final = crypto_sm3_final, .finup = crypto_sm3_finup, .descsize = sizeof(struct sm3_state), .base = { diff --git a/include/crypto/sm3.h b/include/crypto/sm3.h index 1438942dc773..42ea21289ba9 100644 --- a/include/crypto/sm3.h +++ b/include/crypto/sm3.h @@ -35,6 +35,8 @@ struct shash_desc; extern int crypto_sm3_update(struct shash_desc *desc, const u8 *data, unsigned int len); +extern int crypto_sm3_final(struct shash_desc *desc, u8 *out); + extern int crypto_sm3_finup(struct shash_desc *desc, const u8 *data, unsigned int len, u8 *hash); #endif -- 2.19.1.3.ge56e4f7
[PATCH v7 10/10] integrity: Asymmetric digsig supports SM2-with-SM3 algorithm
Asymmetric digsig supports SM2-with-SM3 algorithm combination, so that IMA can also verify SM2's signature data. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang Reviewed-by: Mimi Zohar Reviewed-by: Vitaly Chikunov --- security/integrity/digsig_asymmetric.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index cfa4127d0518..b86a4a8f61ab 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -99,14 +99,22 @@ int asymmetric_verify(struct key *keyring, const char *sig, memset(, 0, sizeof(pks)); pks.hash_algo = hash_algo_name[hdr->hash_algo]; - if (hdr->hash_algo == HASH_ALGO_STREEBOG_256 || - hdr->hash_algo == HASH_ALGO_STREEBOG_512) { + switch (hdr->hash_algo) { + case HASH_ALGO_STREEBOG_256: + case HASH_ALGO_STREEBOG_512: /* EC-RDSA and Streebog should go together. */ pks.pkey_algo = "ecrdsa"; pks.encoding = "raw"; - } else { + break; + case HASH_ALGO_SM3_256: + /* SM2 and SM3 should go together. */ + pks.pkey_algo = "sm2"; + pks.encoding = "raw"; + break; + default: pks.pkey_algo = "rsa"; pks.encoding = "pkcs1"; + break; } pks.digest = (u8 *)data; pks.digest_size = datalen; -- 2.19.1.3.ge56e4f7
[PATCH v7 00/10] crpyto: introduce OSCCA certificate and SM2 asymmetric algorithm
Hello all, This new module implement the OSCCA certificate and SM2 public key algorithm. It was published by State Encryption Management Bureau, China. List of specifications for OSCCA certificate and SM2 elliptic curve public key cryptography: * GM/T 0003.1-2012 * GM/T 0003.2-2012 * GM/T 0003.3-2012 * GM/T 0003.4-2012 * GM/T 0003.5-2012 * GM/T 0015-2012 * GM/T 0009-2012 IETF: https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 oscca: http://www.oscca.gov.cn/sca/xxgk/2010-12/17/content_1002386.shtml scctc: http://www.gmbz.org.cn/main/bzlb.html These patchs add the OID object identifier defined by OSCCA. The x509 certificate supports sm2-with-sm3 type certificate parsing and verification. The sm2 algorithm is based on libgcrypt's mpi implementation, and has made some additions to the kernel's original mpi library, and added the implementation of ec to better support elliptic curve-like algorithms. sm2 has good support in both openssl and gnupg projects, and sm3 and sm4 of the OSCCA algorithm family have also been implemented in the kernel. Among them, sm3 and sm4 have been well implemented in the kernel. This group of patches has newly introduced sm2. In order to implement sm2 more perfectly, I expanded the mpi library and introduced the ec implementation of the mpi library as the basic algorithm. Compared to the kernel's crypto/ecc.c, the implementation of mpi/ec.c is more complete and elegant, sm2 is implemented based on these algorithms. --- v7 changes: 1. add sm2 test vectors to testmgr. 2. fix potential memory leak in testmgr (PATCH 6/10). 3. rebase on mainline. v6 changes: 1. remove mpi_sub_ui function from mpi library. 2. rebase on mainline. v5 changes: 1. fix compilation failure when SM2 is configured as a module. 2. simplify the mpi and ec code, remove unused functions reported by test robot. v4 changes: 1. Pass data directly when calculating sm2 certificate digest. 2. rebase on mainline. v3 changes: 1. integrity asymmetric digsig support sm2-with-sm3 algorithm. 2. remove unused sm2_set_priv_key(). 3. rebase on mainline. v2 changes: 1. simplify the sm2 algorithm and only retain the verify function. 2. extract the sm2 certificate code into a separate file. Tianjia Zhang (10): crypto: sm3 - export crypto_sm3_final function lib/mpi: Extend the MPI library lib/mpi: Introduce ec implementation to MPI library crypto: sm2 - introduce OSCCA SM2 asymmetric cipher algorithm crypto: testmgr - support test with different ciphertext per encryption crypto: testmgr - Fix potential memory leak in test_akcipher_one() crypto: sm2 - add SM2 test vectors to testmgr X.509: support OSCCA certificate parse X.509: support OSCCA SM2-with-SM3 certificate verification integrity: Asymmetric digsig supports SM2-with-SM3 algorithm crypto/Kconfig| 17 + crypto/Makefile |8 + crypto/asymmetric_keys/Makefile |1 + crypto/asymmetric_keys/public_key.c |6 + crypto/asymmetric_keys/public_key_sm2.c | 61 + crypto/asymmetric_keys/x509_cert_parser.c | 27 +- crypto/asymmetric_keys/x509_public_key.c |3 + crypto/sm2.c | 481 +++ crypto/sm2signature.asn1 |4 + crypto/sm3_generic.c |7 +- crypto/testmgr.c | 24 +- crypto/testmgr.h | 59 + include/crypto/public_key.h | 15 + include/crypto/sm2.h | 25 + include/crypto/sm3.h |2 + include/linux/mpi.h | 192 +++ include/linux/oid_registry.h |6 + lib/mpi/Makefile |6 + lib/mpi/ec.c | 1509 + lib/mpi/mpi-add.c | 155 +++ lib/mpi/mpi-bit.c | 251 lib/mpi/mpi-cmp.c | 46 +- lib/mpi/mpi-div.c | 238 lib/mpi/mpi-internal.h| 53 + lib/mpi/mpi-inv.c | 143 ++ lib/mpi/mpi-mod.c | 155 +++ lib/mpi/mpi-mul.c | 94 ++ lib/mpi/mpicoder.c| 336 + lib/mpi/mpih-div.c| 294 lib/mpi/mpih-mul.c| 25 + lib/mpi/mpiutil.c | 204 +++ security/integrity/digsig_asymmetric.c| 14 +- 32 files changed, 4435 insertions(+), 26 deletions(-) create mode 100644 crypto/asymmetric_keys/public_key_sm2.c create mode 100644 crypto/sm2.c create mode 100644 crypto/sm2signature.asn1 create mode 100644 include/crypto/sm2.h create mode 100644 lib/mpi/ec.c create mode 100644 lib/mpi/mpi-add.c create mode 100644 lib/mpi/mpi-div.c create mode 100644 lib/mpi/mpi-inv.c create mode 100644 lib/mpi/mpi-mod.c create mode 100644 lib/mpi/mpi
[PATCH v7 03/10] lib/mpi: Introduce ec implementation to MPI library
The implementation of EC is introduced from libgcrypt as the basic algorithm of elliptic curve, which can be more perfectly integrated with MPI implementation. Some other algorithms will be developed based on mpi ecc, such as SM2. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- include/linux/mpi.h | 105 +++ lib/mpi/Makefile|1 + lib/mpi/ec.c| 1509 +++ 3 files changed, 1615 insertions(+) create mode 100644 lib/mpi/ec.c diff --git a/include/linux/mpi.h b/include/linux/mpi.h index 3c9e41603cf6..3e5358f4de2f 100644 --- a/include/linux/mpi.h +++ b/include/linux/mpi.h @@ -157,6 +157,111 @@ void mpi_fdiv_q(MPI quot, MPI dividend, MPI divisor); /*-- mpi-inv.c --*/ int mpi_invm(MPI x, MPI a, MPI n); +/*-- ec.c --*/ + +/* Object to represent a point in projective coordinates */ +struct gcry_mpi_point { + MPI x; + MPI y; + MPI z; +}; + +typedef struct gcry_mpi_point *MPI_POINT; + +/* Models describing an elliptic curve */ +enum gcry_mpi_ec_models { + /* The Short Weierstrass equation is +* y^2 = x^3 + ax + b +*/ + MPI_EC_WEIERSTRASS = 0, + /* The Montgomery equation is +* by^2 = x^3 + ax^2 + x +*/ + MPI_EC_MONTGOMERY, + /* The Twisted Edwards equation is +* ax^2 + y^2 = 1 + bx^2y^2 +* Note that we use 'b' instead of the commonly used 'd'. +*/ + MPI_EC_EDWARDS +}; + +/* Dialects used with elliptic curves */ +enum ecc_dialects { + ECC_DIALECT_STANDARD = 0, + ECC_DIALECT_ED25519, + ECC_DIALECT_SAFECURVE +}; + +/* This context is used with all our EC functions. */ +struct mpi_ec_ctx { + enum gcry_mpi_ec_models model; /* The model describing this curve. */ + enum ecc_dialects dialect; /* The ECC dialect used with the curve. */ + int flags; /* Public key flags (not always used). */ + unsigned int nbits;/* Number of bits. */ + + /* Domain parameters. Note that they may not all be set and if set +* the MPIs may be flaged as constant. +*/ + MPI p; /* Prime specifying the field GF(p). */ + MPI a; /* First coefficient of the Weierstrass equation. */ + MPI b; /* Second coefficient of the Weierstrass equation. */ + MPI_POINT G; /* Base point (generator). */ + MPI n; /* Order of G. */ + unsigned int h; /* Cofactor. */ + + /* The actual key. May not be set. */ + MPI_POINT Q; /* Public key. */ + MPI d; /* Private key. */ + + const char *name; /* Name of the curve. */ + + /* This structure is private to mpi/ec.c! */ + struct { + struct { + unsigned int a_is_pminus3:1; + unsigned int two_inv_p:1; + } valid; /* Flags to help setting the helper vars below. */ + + int a_is_pminus3; /* True if A = P - 3. */ + + MPI two_inv_p; + + mpi_barrett_t p_barrett; + + /* Scratch variables. */ + MPI scratch[11]; + + /* Helper for fast reduction. */ + /* int nist_nbits; /\* If this is a NIST curve, the # of bits. *\/ */ + /* MPI s[10]; */ + /* MPI c; */ + } t; + + /* Curve specific computation routines for the field. */ + void (*addm)(MPI w, MPI u, MPI v, struct mpi_ec_ctx *ctx); + void (*subm)(MPI w, MPI u, MPI v, struct mpi_ec_ctx *ec); + void (*mulm)(MPI w, MPI u, MPI v, struct mpi_ec_ctx *ctx); + void (*pow2)(MPI w, const MPI b, struct mpi_ec_ctx *ctx); + void (*mul2)(MPI w, MPI u, struct mpi_ec_ctx *ctx); +}; + +void mpi_ec_init(struct mpi_ec_ctx *ctx, enum gcry_mpi_ec_models model, + enum ecc_dialects dialect, + int flags, MPI p, MPI a, MPI b); +void mpi_ec_deinit(struct mpi_ec_ctx *ctx); +MPI_POINT mpi_point_new(unsigned int nbits); +void mpi_point_release(MPI_POINT p); +void mpi_point_init(MPI_POINT p); +void mpi_point_free_parts(MPI_POINT p); +int mpi_ec_get_affine(MPI x, MPI y, MPI_POINT point, struct mpi_ec_ctx *ctx); +void mpi_ec_add_points(MPI_POINT result, + MPI_POINT p1, MPI_POINT p2, + struct mpi_ec_ctx *ctx); +void mpi_ec_mul_point(MPI_POINT result, + MPI scalar, MPI_POINT point, + struct mpi_ec_ctx *ctx); +int mpi_ec_curve_point(MPI_POINT point, struct mpi_ec_ctx *ctx); + /* inline functions */ /** diff --git a/lib/mpi/Makefile b/lib/mpi/Makefile index 477debd7ed50..6e6ef9a34fe1 100644 --- a/lib/mpi/Makefile +++ b/lib/mpi/Makefile @@ -13,6 +13,7 @@ mpi-y = \ generic_mpih-rshift.o \ generic_mpih-sub1.o \ generic_mpih-add1.o \ + ec.o
Re: [PATCH v6 5/8] crypto: testmgr - support test with different ciphertext per encryption
On 9/18/20 2:47 PM, Herbert Xu wrote: On Thu, Sep 03, 2020 at 09:12:39PM +0800, Tianjia Zhang wrote: Some asymmetric algorithms will get different ciphertext after each encryption, such as SM2, and let testmgr support the testing of such algorithms. In struct akcipher_testvec, set c and c_size to be empty, skip the comparison of the ciphertext, and compare the decrypted plaintext with m to achieve the test purpose. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/testmgr.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) Is there supposed to be another patch that contains test vectors? Thanks, Yes, I was negligent. It is necessary to provide a test vector and I will support it as soon as possible. Thanks, Tianjia
Re: [PATCH] vhost-vdpa: fix memory leak in error path
LGTM. Reviewed-by: Tianjia Zhang Thanks. On 9/9/20 11:41 PM, Li Qiang wrote: Free the 'page_list' when the 'npages' is zero. Signed-off-by: Li Qiang --- drivers/vhost/vdpa.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 3fab94f88894..6a9fcaf1831d 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -609,8 +609,10 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, gup_flags |= FOLL_WRITE; npages = PAGE_ALIGN(msg->size + (iova & ~PAGE_MASK)) >> PAGE_SHIFT; - if (!npages) - return -EINVAL; + if (!npages) { + ret = -EINVAL; + goto free_page; + } mmap_read_lock(dev->mm); @@ -666,6 +668,8 @@ static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v, atomic64_sub(npages, >mm->pinned_vm); } mmap_read_unlock(dev->mm); + +free_page: free_page((unsigned long)page_list); return ret; }
Re: [PATCH v1] atomisp:pci/runtime/queue: modify the return error value
LGTM. Reviewed-by: Tianjia Zhang On 9/17/20 11:44 AM, Xiaoliang Pang wrote: modify the return error value is -EDOM Fixes: 2cac05dee6e30("drm/amd/powerplay: add the hw manager for vega12 (v4)") Cc: Evan Quan Signed-off-by: Xiaoliang Pang --- .../staging/media/atomisp/pci/runtime/queue/src/queue_access.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c index fdca743c4ab7..424e7a15a389 100644 --- a/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c +++ b/drivers/staging/media/atomisp/pci/runtime/queue/src/queue_access.c @@ -44,7 +44,7 @@ int ia_css_queue_load( the value as zero. This causes division by 0 exception as the size is used in a modular division operation. */ - return EDOM; + return -EDOM; } }
Re: [PATCH v6 0/8] crpyto: introduce OSCCA certificate and SM2 asymmetric algorithm
Hi, On 9/11/20 12:24 PM, Herbert Xu wrote: On Thu, Sep 03, 2020 at 09:12:34PM +0800, Tianjia Zhang wrote: --- v6 changes: 1. remove mpi_sub_ui function from mpi library. 2. rebase on mainline. This series is still missing acks for patches 6-8. Without them it cannot proceed. Thanks, This series has some review suggestions for patches 6-8, but the maintainer of asymmetric keys David Howells , I can’t get in touch. The email has been sent successfully. Can you help ping him ? Thanks and best, Tianjia
Re: [PATCH v6 7/8] X.509: support OSCCA sm2-with-sm3 certificate verification
Hi Gilad, On 9/13/20 3:12 PM, Gilad Ben-Yossef wrote: Hi, On Thu, Sep 3, 2020 at 4:13 PM Tianjia Zhang wrote: The digital certificate format based on SM2 crypto algorithm as specified in GM/T 0015-2012. It was published by State Encryption Management Bureau, China. The method of generating Other User Information is defined as ZA=H256(ENTLA || IDA || a || b || xG || yG || xA || yA), it also specified in https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02. The x509 certificate supports sm2-with-sm3 type certificate verification. Because certificate verification requires ZA in addition to tbs data, ZA also depends on elliptic curve parameters and public key data, so you need to access tbs in sig and calculate ZA. Finally calculate the digest of the signature and complete the verification work. The calculation process of ZA is declared in specifications GM/T 0009-2012 and GM/T 0003.2-2012. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/asymmetric_keys/Makefile | 1 + crypto/asymmetric_keys/public_key.c | 6 +++ crypto/asymmetric_keys/public_key_sm2.c | 61 crypto/asymmetric_keys/x509_public_key.c | 3 ++ include/crypto/public_key.h | 15 ++ 5 files changed, 86 insertions(+) create mode 100644 crypto/asymmetric_keys/public_key_sm2.c diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile index 28b91adba2ae..1a99ea5acb6b 100644 --- a/crypto/asymmetric_keys/Makefile +++ b/crypto/asymmetric_keys/Makefile @@ -11,6 +11,7 @@ asymmetric_keys-y := \ signature.o obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key_sm2.o obj-$(CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE) += asym_tpm.o # diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index d8410ffd7f12..1d0492098bbd 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -299,6 +299,12 @@ int public_key_verify_signature(const struct public_key *pkey, if (ret) goto error_free_key; + if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { + ret = cert_sig_digest_update(sig, tfm); + if (ret) + goto error_free_key; + } + sg_init_table(src_sg, 2); sg_set_buf(_sg[0], sig->s, sig->s_size); sg_set_buf(_sg[1], sig->digest, sig->digest_size); diff --git a/crypto/asymmetric_keys/public_key_sm2.c b/crypto/asymmetric_keys/public_key_sm2.c new file mode 100644 index ..7325cf21dbb4 --- /dev/null +++ b/crypto/asymmetric_keys/public_key_sm2.c @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * asymmetric public-key algorithm for SM2-with-SM3 certificate + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + * + * Copyright (c) 2020, Alibaba Group. + * Authors: Tianjia Zhang + */ + +#include +#include +#include + +#if IS_REACHABLE(CONFIG_CRYPTO_SM2) + +int cert_sig_digest_update(const struct public_key_signature *sig, + struct crypto_akcipher *tfm_pkey) +{ + struct crypto_shash *tfm; + struct shash_desc *desc; + size_t desc_size; + unsigned char dgst[SM3_DIGEST_SIZE]; + int ret; + + BUG_ON(!sig->data); + + ret = sm2_compute_z_digest(tfm_pkey, SM2_DEFAULT_USERID, + SM2_DEFAULT_USERID_LEN, dgst); + if (ret) + return ret; + + tfm = crypto_alloc_shash(sig->hash_algo, 0, 0); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); + + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); + desc = kzalloc(desc_size, GFP_KERNEL); + if (!desc) + goto error_free_tfm; + + desc->tfm = tfm; + + ret = crypto_shash_init(desc); + if (ret < 0) + goto error_free_desc; + + ret = crypto_shash_update(desc, dgst, SM3_DIGEST_SIZE); + if (ret < 0) + goto error_free_desc; + + ret = crypto_shash_finup(desc, sig->data, sig->data_size, sig->digest); It looks like you are doing a separate init, update, finup every time - I would consider using crypto_shash_digest() in one go. In fact, considering the fact that you are allocating a tfm just for this use and then releasing it, I would consider switching to crypto_shash_tfm_digest() and dropping the kzalloc all together. This should simplify the code a bit. Other than that I don't have anything smart to say :-) Gilad The hash calculation here includes two parts of data, 'dgst' and 'sig->data'. The last call is 'finup()' not 'final()'. I understand that it should not be possible to use 'crypto_shash_tfm_digest()' This kind of function is simplified. I
[PATCH v6 6/8] X.509: support OSCCA certificate parse
The digital certificate format based on SM2 crypto algorithm as specified in GM/T 0015-2012. It was published by State Encryption Management Bureau, China. This patch adds the OID object identifier defined by OSCCA. The x509 certificate supports sm2-with-sm3 type certificate parsing. It uses the standard elliptic curve public key, and the sm2 algorithm signs the hash generated by sm3. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/asymmetric_keys/x509_cert_parser.c | 14 +- include/linux/oid_registry.h | 6 ++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 26ec20ef4899..6a8aee22bfd4 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -234,6 +234,10 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, case OID_gost2012Signature512: ctx->cert->sig->hash_algo = "streebog512"; goto ecrdsa; + + case OID_sm2_with_sm3: + ctx->cert->sig->hash_algo = "sm3"; + goto sm2; } rsa_pkcs1: @@ -246,6 +250,11 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, ctx->cert->sig->encoding = "raw"; ctx->algo_oid = ctx->last_oid; return 0; +sm2: + ctx->cert->sig->pkey_algo = "sm2"; + ctx->cert->sig->encoding = "raw"; + ctx->algo_oid = ctx->last_oid; + return 0; } /* @@ -266,7 +275,8 @@ int x509_note_signature(void *context, size_t hdrlen, } if (strcmp(ctx->cert->sig->pkey_algo, "rsa") == 0 || - strcmp(ctx->cert->sig->pkey_algo, "ecrdsa") == 0) { + strcmp(ctx->cert->sig->pkey_algo, "ecrdsa") == 0 || + strcmp(ctx->cert->sig->pkey_algo, "sm2") == 0) { /* Discard the BIT STRING metadata */ if (vlen < 1 || *(const u8 *)value != 0) return -EBADMSG; @@ -456,6 +466,8 @@ int x509_extract_key_data(void *context, size_t hdrlen, else if (ctx->last_oid == OID_gost2012PKey256 || ctx->last_oid == OID_gost2012PKey512) ctx->cert->pub->pkey_algo = "ecrdsa"; + else if (ctx->last_oid == OID_id_ecPublicKey) + ctx->cert->pub->pkey_algo = "sm2"; else return -ENOPKG; diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index 657d6bf2c064..48fe3133ff39 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -107,6 +107,12 @@ enum OID { OID_gostTC26Sign512B, /* 1.2.643.7.1.2.1.2.2 */ OID_gostTC26Sign512C, /* 1.2.643.7.1.2.1.2.3 */ + /* OSCCA */ + OID_sm2,/* 1.2.156.10197.1.301 */ + OID_sm3,/* 1.2.156.10197.1.401 */ + OID_sm2_with_sm3, /* 1.2.156.10197.1.501 */ + OID_sm3WithRSAEncryption, /* 1.2.156.10197.1.504 */ + OID__NR }; -- 2.19.1.3.ge56e4f7
[PATCH v6 4/8] crypto: sm2 - introduce OSCCA SM2 asymmetric cipher algorithm
This new module implement the SM2 public key algorithm. It was published by State Encryption Management Bureau, China. List of specifications for SM2 elliptic curve public key cryptography: * GM/T 0003.1-2012 * GM/T 0003.2-2012 * GM/T 0003.3-2012 * GM/T 0003.4-2012 * GM/T 0003.5-2012 IETF: https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 oscca: http://www.oscca.gov.cn/sca/xxgk/2010-12/17/content_1002386.shtml scctc: http://www.gmbz.org.cn/main/bzlb.html Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/Kconfig | 17 ++ crypto/Makefile | 8 + crypto/sm2.c | 473 +++ crypto/sm2signature.asn1 | 4 + include/crypto/sm2.h | 25 +++ 5 files changed, 527 insertions(+) create mode 100644 crypto/sm2.c create mode 100644 crypto/sm2signature.asn1 create mode 100644 include/crypto/sm2.h diff --git a/crypto/Kconfig b/crypto/Kconfig index 1b57419fa2e7..fd1b2693aca2 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -260,6 +260,23 @@ config CRYPTO_ECRDSA standard algorithms (called GOST algorithms). Only signature verification is implemented. +config CRYPTO_SM2 + tristate "SM2 algorithm" + select CRYPTO_SM3 + select CRYPTO_AKCIPHER + select CRYPTO_MANAGER + select MPILIB + select ASN1 + help + Generic implementation of the SM2 public key algorithm. It was + published by State Encryption Management Bureau, China. + as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012. + + References: + https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + http://www.oscca.gov.cn/sca/xxgk/2010-12/17/content_1002386.shtml + http://www.gmbz.org.cn/main/bzlb.html + config CRYPTO_CURVE25519 tristate "Curve25519 algorithm" select CRYPTO_KPP diff --git a/crypto/Makefile b/crypto/Makefile index 4ca12b6044f7..b279483fba50 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -42,6 +42,14 @@ rsa_generic-y += rsa_helper.o rsa_generic-y += rsa-pkcs1pad.o obj-$(CONFIG_CRYPTO_RSA) += rsa_generic.o +$(obj)/sm2signature.asn1.o: $(obj)/sm2signature.asn1.c $(obj)/sm2signature.asn1.h +$(obj)/sm2.o: $(obj)/sm2signature.asn1.h + +sm2_generic-y += sm2signature.asn1.o +sm2_generic-y += sm2.o + +obj-$(CONFIG_CRYPTO_SM2) += sm2_generic.o + crypto_acompress-y := acompress.o crypto_acompress-y += scompress.o obj-$(CONFIG_CRYPTO_ACOMP2) += crypto_acompress.o diff --git a/crypto/sm2.c b/crypto/sm2.c new file mode 100644 index ..86da175bcda6 --- /dev/null +++ b/crypto/sm2.c @@ -0,0 +1,473 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * SM2 asymmetric public-key algorithm + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + * + * Copyright (c) 2020, Alibaba Group. + * Authors: Tianjia Zhang + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include "sm2signature.asn1.h" + +#define MPI_NBYTES(m) ((mpi_get_nbits(m) + 7) / 8) + +struct ecc_domain_parms { + const char *desc; /* Description of the curve. */ + unsigned int nbits; /* Number of bits. */ + unsigned int fips:1; /* True if this is a FIPS140-2 approved curve */ + + /* The model describing this curve. This is mainly used to select +* the group equation. +*/ + enum gcry_mpi_ec_models model; + + /* The actual ECC dialect used. This is used for curve specific +* optimizations and to select encodings etc. +*/ + enum ecc_dialects dialect; + + const char *p; /* The prime defining the field. */ + const char *a, *b; /* The coefficients. For Twisted Edwards +* Curves b is used for d. For Montgomery +* Curves (a,b) has ((A-2)/4,B^-1). +*/ + const char *n; /* The order of the base point. */ + const char *g_x, *g_y; /* Base point. */ + unsigned int h; /* Cofactor. */ +}; + +static const struct ecc_domain_parms sm2_ecp = { + .desc = "sm2p256v1", + .nbits = 256, + .fips = 0, + .model = MPI_EC_WEIERSTRASS, + .dialect = ECC_DIALECT_STANDARD, + .p = "0xfffe", + .a = "0xfffefffc", + .b = "0x28e9fa9e9d9f5e344d5a9e4bcf6509a7f39789f515ab8f92ddbcbd414d940e93", + .n = "0xfffe7203df6b21c6052b53bbf40939d54123", + .g_x = "0x32c4ae2c1f1981195f9904466a39c9948fe30bbff2660be1715a4589334c74c7", + .g_y = "0xbc3736a2f4f6779c59bdcee36b692153d0a987
[PATCH v6 7/8] X.509: support OSCCA sm2-with-sm3 certificate verification
The digital certificate format based on SM2 crypto algorithm as specified in GM/T 0015-2012. It was published by State Encryption Management Bureau, China. The method of generating Other User Information is defined as ZA=H256(ENTLA || IDA || a || b || xG || yG || xA || yA), it also specified in https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02. The x509 certificate supports sm2-with-sm3 type certificate verification. Because certificate verification requires ZA in addition to tbs data, ZA also depends on elliptic curve parameters and public key data, so you need to access tbs in sig and calculate ZA. Finally calculate the digest of the signature and complete the verification work. The calculation process of ZA is declared in specifications GM/T 0009-2012 and GM/T 0003.2-2012. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/asymmetric_keys/Makefile | 1 + crypto/asymmetric_keys/public_key.c | 6 +++ crypto/asymmetric_keys/public_key_sm2.c | 61 crypto/asymmetric_keys/x509_public_key.c | 3 ++ include/crypto/public_key.h | 15 ++ 5 files changed, 86 insertions(+) create mode 100644 crypto/asymmetric_keys/public_key_sm2.c diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile index 28b91adba2ae..1a99ea5acb6b 100644 --- a/crypto/asymmetric_keys/Makefile +++ b/crypto/asymmetric_keys/Makefile @@ -11,6 +11,7 @@ asymmetric_keys-y := \ signature.o obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key_sm2.o obj-$(CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE) += asym_tpm.o # diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index d8410ffd7f12..1d0492098bbd 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -299,6 +299,12 @@ int public_key_verify_signature(const struct public_key *pkey, if (ret) goto error_free_key; + if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { + ret = cert_sig_digest_update(sig, tfm); + if (ret) + goto error_free_key; + } + sg_init_table(src_sg, 2); sg_set_buf(_sg[0], sig->s, sig->s_size); sg_set_buf(_sg[1], sig->digest, sig->digest_size); diff --git a/crypto/asymmetric_keys/public_key_sm2.c b/crypto/asymmetric_keys/public_key_sm2.c new file mode 100644 index ..7325cf21dbb4 --- /dev/null +++ b/crypto/asymmetric_keys/public_key_sm2.c @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * asymmetric public-key algorithm for SM2-with-SM3 certificate + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 + * + * Copyright (c) 2020, Alibaba Group. + * Authors: Tianjia Zhang + */ + +#include +#include +#include + +#if IS_REACHABLE(CONFIG_CRYPTO_SM2) + +int cert_sig_digest_update(const struct public_key_signature *sig, + struct crypto_akcipher *tfm_pkey) +{ + struct crypto_shash *tfm; + struct shash_desc *desc; + size_t desc_size; + unsigned char dgst[SM3_DIGEST_SIZE]; + int ret; + + BUG_ON(!sig->data); + + ret = sm2_compute_z_digest(tfm_pkey, SM2_DEFAULT_USERID, + SM2_DEFAULT_USERID_LEN, dgst); + if (ret) + return ret; + + tfm = crypto_alloc_shash(sig->hash_algo, 0, 0); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); + + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); + desc = kzalloc(desc_size, GFP_KERNEL); + if (!desc) + goto error_free_tfm; + + desc->tfm = tfm; + + ret = crypto_shash_init(desc); + if (ret < 0) + goto error_free_desc; + + ret = crypto_shash_update(desc, dgst, SM3_DIGEST_SIZE); + if (ret < 0) + goto error_free_desc; + + ret = crypto_shash_finup(desc, sig->data, sig->data_size, sig->digest); + +error_free_desc: + kfree(desc); +error_free_tfm: + crypto_free_shash(tfm); + return ret; +} + +#endif /* ! IS_REACHABLE(CONFIG_CRYPTO_SM2) */ diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index d964cc82b69c..ae450eb8be14 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -30,6 +30,9 @@ int x509_get_sig_params(struct x509_certificate *cert) pr_devel("==>%s()\n", __func__); + sig->data = cert->tbs; + sig->data_size = cert->tbs_size; + if (!cert->pub->pkey_algo) cert->unsupported_key = true; diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h index 11f535cfb810..02a6dbe5
[PATCH v6 5/8] crypto: testmgr - support test with different ciphertext per encryption
Some asymmetric algorithms will get different ciphertext after each encryption, such as SM2, and let testmgr support the testing of such algorithms. In struct akcipher_testvec, set c and c_size to be empty, skip the comparison of the ciphertext, and compare the decrypted plaintext with m to achieve the test purpose. Signed-off-by: Tianjia Zhang Tested-by: Xufeng Zhang --- crypto/testmgr.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 23c27fc96394..cd002a030af5 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -4025,7 +4025,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, pr_err("alg: akcipher: %s test failed. err %d\n", op, err); goto free_all; } - if (!vecs->siggen_sigver_test) { + if (!vecs->siggen_sigver_test && c) { if (req->dst_len != c_size) { pr_err("alg: akcipher: %s test failed. Invalid output len\n", op); @@ -4056,6 +4056,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, goto free_all; } + if (!vecs->siggen_sigver_test && !c) { + c = outbuf_enc; + c_size = req->dst_len; + } + op = vecs->siggen_sigver_test ? "sign" : "decrypt"; if (WARN_ON(c_size > PAGE_SIZE)) goto free_all; -- 2.19.1.3.ge56e4f7
[PATCH v6 0/8] crpyto: introduce OSCCA certificate and SM2 asymmetric algorithm
Hello all, This new module implement the OSCCA certificate and SM2 public key algorithm. It was published by State Encryption Management Bureau, China. List of specifications for OSCCA certificate and SM2 elliptic curve public key cryptography: * GM/T 0003.1-2012 * GM/T 0003.2-2012 * GM/T 0003.3-2012 * GM/T 0003.4-2012 * GM/T 0003.5-2012 * GM/T 0015-2012 * GM/T 0009-2012 IETF: https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02 oscca: http://www.oscca.gov.cn/sca/xxgk/2010-12/17/content_1002386.shtml scctc: http://www.gmbz.org.cn/main/bzlb.html These patchs add the OID object identifier defined by OSCCA. The x509 certificate supports sm2-with-sm3 type certificate parsing and verification. The sm2 algorithm is based on libgcrypt's mpi implementation, and has made some additions to the kernel's original mpi library, and added the implementation of ec to better support elliptic curve-like algorithms. sm2 has good support in both openssl and gnupg projects, and sm3 and sm4 of the OSCCA algorithm family have also been implemented in the kernel. Among them, sm3 and sm4 have been well implemented in the kernel. This group of patches has newly introduced sm2. In order to implement sm2 more perfectly, I expanded the mpi library and introduced the ec implementation of the mpi library as the basic algorithm. Compared to the kernel's crypto/ecc.c, the implementation of mpi/ec.c is more complete and elegant, sm2 is implemented based on these algorithms. --- v6 changes: 1. remove mpi_sub_ui function from mpi library. 2. rebase on mainline. v5 changes: 1. fix compilation failure when SM2 is configured as a module. 2. simplify the mpi and ec code, remove unused functions reported by test robot. v4 changes: 1. Pass data directly when calculating sm2 certificate digest. 2. rebase on mainline. v3 changes: 1. integrity asymmetric digsig support sm2-with-sm3 algorithm. 2. remove unused sm2_set_priv_key(). 3. rebase on mainline. v2 changes: 1. simplify the sm2 algorithm and only retain the verify function. 2. extract the sm2 certificate code into a separate file. Tianjia Zhang (8): crypto: sm3 - export crypto_sm3_final function lib/mpi: Extend the MPI library lib/mpi: Introduce ec implementation to MPI library crypto: sm2 - introduce OSCCA SM2 asymmetric cipher algorithm crypto: testmgr - support test with different ciphertext per encryption X.509: support OSCCA certificate parse X.509: support OSCCA sm2-with-sm3 certificate verification integrity: Asymmetric digsig supports SM2-with-SM3 algorithm crypto/Kconfig| 17 + crypto/Makefile |8 + crypto/asymmetric_keys/Makefile |1 + crypto/asymmetric_keys/public_key.c |6 + crypto/asymmetric_keys/public_key_sm2.c | 61 + crypto/asymmetric_keys/x509_cert_parser.c | 14 +- crypto/asymmetric_keys/x509_public_key.c |3 + crypto/sm2.c | 473 +++ crypto/sm2signature.asn1 |4 + crypto/sm3_generic.c |7 +- crypto/testmgr.c |7 +- include/crypto/public_key.h | 15 + include/crypto/sm2.h | 25 + include/crypto/sm3.h |2 + include/linux/mpi.h | 192 +++ include/linux/oid_registry.h |6 + lib/mpi/Makefile |6 + lib/mpi/ec.c | 1509 + lib/mpi/mpi-add.c | 155 +++ lib/mpi/mpi-bit.c | 251 lib/mpi/mpi-cmp.c | 46 +- lib/mpi/mpi-div.c | 238 lib/mpi/mpi-internal.h| 53 + lib/mpi/mpi-inv.c | 143 ++ lib/mpi/mpi-mod.c | 155 +++ lib/mpi/mpi-mul.c | 94 ++ lib/mpi/mpicoder.c| 336 + lib/mpi/mpih-div.c| 294 lib/mpi/mpih-mul.c| 25 + lib/mpi/mpiutil.c | 204 +++ security/integrity/digsig_asymmetric.c| 14 +- 31 files changed, 4346 insertions(+), 18 deletions(-) create mode 100644 crypto/asymmetric_keys/public_key_sm2.c create mode 100644 crypto/sm2.c create mode 100644 crypto/sm2signature.asn1 create mode 100644 include/crypto/sm2.h create mode 100644 lib/mpi/ec.c create mode 100644 lib/mpi/mpi-add.c create mode 100644 lib/mpi/mpi-div.c create mode 100644 lib/mpi/mpi-inv.c create mode 100644 lib/mpi/mpi-mod.c create mode 100644 lib/mpi/mpi-mul.c -- 2.19.1.3.ge56e4f7