Re: [PATCH v2 0/2] support sign module with SM2-with-SM3 algorithm

2021-04-06 Thread Tianjia Zhang

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

2021-04-06 Thread Tianjia Zhang




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

2021-03-25 Thread Tianjia Zhang
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

2021-03-24 Thread Tianjia Zhang
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

2021-03-24 Thread Tianjia Zhang
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

2021-03-24 Thread Tianjia Zhang
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

2021-03-24 Thread tip-bot2 for Tianjia Zhang
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

2021-03-24 Thread Tianjia Zhang

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

2021-03-24 Thread Tianjia Zhang

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

2021-03-24 Thread Tianjia Zhang

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

2021-03-23 Thread Tianjia Zhang
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

2021-03-22 Thread Tianjia Zhang

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

2021-03-22 Thread Tianjia Zhang
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

2021-03-14 Thread Tianjia Zhang
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

2021-03-10 Thread Tianjia Zhang




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

2021-03-07 Thread Tianjia Zhang

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

2021-03-04 Thread Tianjia Zhang

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

2021-03-04 Thread Tianjia Zhang

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

2021-03-03 Thread Tianjia Zhang




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

2021-03-03 Thread Tianjia Zhang




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

2021-03-02 Thread Tianjia Zhang




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

2021-02-28 Thread Tianjia Zhang
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

2021-02-24 Thread Tianjia Zhang




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

2021-02-15 Thread Tianjia Zhang
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

2021-02-15 Thread Tianjia Zhang
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

2021-02-15 Thread Tianjia Zhang
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

2021-02-15 Thread Tianjia Zhang
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

2021-02-15 Thread Tianjia Zhang




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

2021-02-10 Thread Tianjia Zhang




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()

2021-02-10 Thread Tianjia Zhang




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

2021-02-10 Thread Tianjia Zhang




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

2021-02-10 Thread Tianjia Zhang




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

2021-02-10 Thread Tianjia Zhang

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

2021-02-01 Thread Tianjia Zhang
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

2021-02-01 Thread Tianjia Zhang
'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()

2021-02-01 Thread Tianjia Zhang
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

2021-02-01 Thread Tianjia Zhang
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

2021-02-01 Thread Tianjia Zhang
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

2021-02-01 Thread Tianjia Zhang
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

2021-02-01 Thread Tianjia Zhang




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

2021-02-01 Thread Tianjia Zhang




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

2021-02-01 Thread Tianjia Zhang

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

2021-02-01 Thread Tianjia Zhang




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

2021-02-01 Thread Tianjia Zhang




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

2021-01-23 Thread Tianjia Zhang




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

2021-01-23 Thread Tianjia Zhang




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

2021-01-23 Thread Tianjia Zhang
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

2021-01-23 Thread Tianjia Zhang
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()

2021-01-23 Thread Tianjia Zhang
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

2021-01-23 Thread Tianjia Zhang
`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

2021-01-23 Thread Tianjia Zhang
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

2021-01-23 Thread Tianjia Zhang
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

2021-01-19 Thread Tianjia Zhang




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

2021-01-19 Thread Tianjia Zhang
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

2021-01-19 Thread Tianjia Zhang

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

2021-01-19 Thread Tianjia Zhang
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

2021-01-19 Thread Tianjia Zhang
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()

2021-01-18 Thread Tianjia Zhang
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

2021-01-18 Thread Tianjia Zhang
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

2021-01-18 Thread Tianjia Zhang
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

2021-01-18 Thread Tianjia Zhang
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

2021-01-18 Thread Tianjia Zhang
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

2021-01-13 Thread Tianjia Zhang




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

2021-01-13 Thread Tianjia Zhang
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

2021-01-12 Thread Tianjia Zhang




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

2021-01-07 Thread Tianjia Zhang
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

2020-11-29 Thread Tianjia Zhang




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

2020-11-29 Thread Tianjia Zhang

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

2020-11-18 Thread Tianjia Zhang
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

2020-10-15 Thread Tianjia Zhang
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

2020-10-15 Thread Tianjia Zhang
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

2020-10-05 Thread Tianjia Zhang
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()

2020-10-05 Thread Tianjia Zhang
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

2020-10-05 Thread Tianjia Zhang
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()

2020-09-27 Thread tip-bot2 for Tianjia Zhang
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

2020-09-25 Thread Tianjia Zhang

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

2020-09-25 Thread Tianjia Zhang

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

2020-09-21 Thread Tianjia Zhang




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

2020-09-20 Thread Tianjia Zhang
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()

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-20 Thread Tianjia Zhang
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

2020-09-18 Thread Tianjia Zhang




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

2020-09-18 Thread Tianjia Zhang

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

2020-09-16 Thread Tianjia Zhang

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

2020-09-14 Thread Tianjia Zhang

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

2020-09-14 Thread Tianjia Zhang

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

2020-09-03 Thread Tianjia Zhang
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

2020-09-03 Thread Tianjia Zhang
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

2020-09-03 Thread Tianjia Zhang
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

2020-09-03 Thread Tianjia Zhang
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

2020-09-03 Thread Tianjia Zhang
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



  1   2   >