2021-10-09 13:09:02

by Tianjia Zhang

[permalink] [raw]
Subject: [PATCH 0/2] tpm: use SM3 instead of SM3_256

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Tianjia Zhang (2):
crypto: use SM3 instead of SM3_256
tpm: use SM3 instead of SM3_256

Documentation/security/keys/trusted-encrypted.rst | 2 +-
crypto/hash_info.c | 4 ++--
drivers/char/tpm/tpm-sysfs.c | 4 ++--
drivers/char/tpm/tpm2-cmd.c | 2 +-
include/crypto/hash_info.h | 2 +-
include/linux/tpm.h | 2 +-
include/uapi/linux/hash_info.h | 2 +-
security/keys/trusted-keys/trusted_tpm2.c | 2 +-
8 files changed, 10 insertions(+), 10 deletions(-)

--
2.19.1.3.ge56e4f7


2021-10-09 13:09:02

by Tianjia Zhang

[permalink] [raw]
Subject: [PATCH 1/2] crypto: use SM3 instead of SM3_256

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Signed-off-by: Tianjia Zhang <[email protected]>
---
Documentation/security/keys/trusted-encrypted.rst | 2 +-
crypto/hash_info.c | 4 ++--
drivers/char/tpm/tpm2-cmd.c | 2 +-
include/crypto/hash_info.h | 2 +-
include/uapi/linux/hash_info.h | 2 +-
security/keys/trusted-keys/trusted_tpm2.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 80d5a5af62a1..3292461517f6 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -162,7 +162,7 @@ Usage::
default 1 (resealing allowed)
hash= hash algorithm name as a string. For TPM 1.x the only
allowed value is sha1. For TPM 2.x the allowed values
- are sha1, sha256, sha384, sha512 and sm3-256.
+ are sha1, sha256, sha384, sha512 and sm3.
policydigest= digest for the authorization policy. must be calculated
with the same hash algorithm as specified by the 'hash='
option.
diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index a49ff96bde77..fe0119407219 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -26,7 +26,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
[HASH_ALGO_TGR_128] = "tgr128",
[HASH_ALGO_TGR_160] = "tgr160",
[HASH_ALGO_TGR_192] = "tgr192",
- [HASH_ALGO_SM3_256] = "sm3",
+ [HASH_ALGO_SM3] = "sm3",
[HASH_ALGO_STREEBOG_256] = "streebog256",
[HASH_ALGO_STREEBOG_512] = "streebog512",
};
@@ -50,7 +50,7 @@ const int hash_digest_size[HASH_ALGO__LAST] = {
[HASH_ALGO_TGR_128] = TGR128_DIGEST_SIZE,
[HASH_ALGO_TGR_160] = TGR160_DIGEST_SIZE,
[HASH_ALGO_TGR_192] = TGR192_DIGEST_SIZE,
- [HASH_ALGO_SM3_256] = SM3256_DIGEST_SIZE,
+ [HASH_ALGO_SM3] = SM3_DIGEST_SIZE,
[HASH_ALGO_STREEBOG_256] = STREEBOG256_DIGEST_SIZE,
[HASH_ALGO_STREEBOG_512] = STREEBOG512_DIGEST_SIZE,
};
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a25815a6f625..20f55de9d87b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -19,7 +19,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
{HASH_ALGO_SHA256, TPM_ALG_SHA256},
{HASH_ALGO_SHA384, TPM_ALG_SHA384},
{HASH_ALGO_SHA512, TPM_ALG_SHA512},
- {HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+ {HASH_ALGO_SM3, TPM_ALG_SM3_256},
};

int tpm2_get_timeouts(struct tpm_chip *chip)
diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h
index dd4f06785049..c1e6b2884732 100644
--- a/include/crypto/hash_info.h
+++ b/include/crypto/hash_info.h
@@ -32,7 +32,7 @@
#define TGR192_DIGEST_SIZE 24

/* not defined in include/crypto/ */
-#define SM3256_DIGEST_SIZE 32
+#define SM3_DIGEST_SIZE 32

extern const char *const hash_algo_name[HASH_ALGO__LAST];
extern const int hash_digest_size[HASH_ALGO__LAST];
diff --git a/include/uapi/linux/hash_info.h b/include/uapi/linux/hash_info.h
index 74a8609fcb4d..1355525dd4aa 100644
--- a/include/uapi/linux/hash_info.h
+++ b/include/uapi/linux/hash_info.h
@@ -32,7 +32,7 @@ enum hash_algo {
HASH_ALGO_TGR_128,
HASH_ALGO_TGR_160,
HASH_ALGO_TGR_192,
- HASH_ALGO_SM3_256,
+ HASH_ALGO_SM3,
HASH_ALGO_STREEBOG_256,
HASH_ALGO_STREEBOG_512,
HASH_ALGO__LAST
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 0165da386289..52a696035176 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -23,7 +23,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
{HASH_ALGO_SHA256, TPM_ALG_SHA256},
{HASH_ALGO_SHA384, TPM_ALG_SHA384},
{HASH_ALGO_SHA512, TPM_ALG_SHA512},
- {HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+ {HASH_ALGO_SM3, TPM_ALG_SM3_256},
};

static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
--
2.19.1.3.ge56e4f7

2021-10-09 13:09:02

by Tianjia Zhang

[permalink] [raw]
Subject: [PATCH 2/2] tpm: use SM3 instead of SM3_256

According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
SM3 always produces a 256-bit hash value and there are no plans for
other length development, so there is no ambiguity in the name of sm3.

Signed-off-by: Tianjia Zhang <[email protected]>
---
drivers/char/tpm/tpm-sysfs.c | 4 ++--
drivers/char/tpm/tpm2-cmd.c | 2 +-
include/linux/tpm.h | 2 +-
security/keys/trusted-keys/trusted_tpm2.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 63f03cfb8e6a..fe6c785dc84a 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -471,7 +471,7 @@ PCR_ATTR_BUILD(TPM_ALG_SHA1, sha1);
PCR_ATTR_BUILD(TPM_ALG_SHA256, sha256);
PCR_ATTR_BUILD(TPM_ALG_SHA384, sha384);
PCR_ATTR_BUILD(TPM_ALG_SHA512, sha512);
-PCR_ATTR_BUILD(TPM_ALG_SM3_256, sm3);
+PCR_ATTR_BUILD(TPM_ALG_SM3, sm3);


void tpm_sysfs_add_device(struct tpm_chip *chip)
@@ -500,7 +500,7 @@ void tpm_sysfs_add_device(struct tpm_chip *chip)
case TPM_ALG_SHA512:
chip->groups[chip->groups_cnt++] = &pcr_group_sha512;
break;
- case TPM_ALG_SM3_256:
+ case TPM_ALG_SM3:
chip->groups[chip->groups_cnt++] = &pcr_group_sm3;
break;
default:
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 20f55de9d87b..d5a9410d2273 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -19,7 +19,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
{HASH_ALGO_SHA256, TPM_ALG_SHA256},
{HASH_ALGO_SHA384, TPM_ALG_SHA384},
{HASH_ALGO_SHA512, TPM_ALG_SHA512},
- {HASH_ALGO_SM3, TPM_ALG_SM3_256},
+ {HASH_ALGO_SM3, TPM_ALG_SM3},
};

int tpm2_get_timeouts(struct tpm_chip *chip)
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index aa11fe323c56..56a79fee1250 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -40,7 +40,7 @@ enum tpm_algorithms {
TPM_ALG_SHA384 = 0x000C,
TPM_ALG_SHA512 = 0x000D,
TPM_ALG_NULL = 0x0010,
- TPM_ALG_SM3_256 = 0x0012,
+ TPM_ALG_SM3 = 0x0012,
};

/*
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 52a696035176..b15a9961213d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -23,7 +23,7 @@ static struct tpm2_hash tpm2_hash_map[] = {
{HASH_ALGO_SHA256, TPM_ALG_SHA256},
{HASH_ALGO_SHA384, TPM_ALG_SHA384},
{HASH_ALGO_SHA512, TPM_ALG_SHA512},
- {HASH_ALGO_SM3, TPM_ALG_SM3_256},
+ {HASH_ALGO_SM3, TPM_ALG_SM3},
};

static u32 tpm2key_oid[] = { 2, 23, 133, 10, 1, 5 };
--
2.19.1.3.ge56e4f7

2021-10-09 13:31:15

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 0/2] tpm: use SM3 instead of SM3_256

On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> SM3 always produces a 256-bit hash value and there are no plans for
> other length development, so there is no ambiguity in the name of
> sm3.

For the TPM we're following the TPM Library specification

https://trustedcomputinggroup.org/resource/tpm-library-specification/

Which is very clear: the algorithm name is TPM_ALG_SM3_256

We're using sm3 as our exposed name because that's what linux crypto
uses, so there should be no problem in what the end user sees, but
changing to non standard TPM definitions is only going to cause
confusion at the kernel level.

James


2021-10-11 07:07:24

by Tianjia Zhang

[permalink] [raw]
Subject: Re: [PATCH 0/2] tpm: use SM3 instead of SM3_256

Hi James,

On 10/9/21 9:29 PM, James Bottomley wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
>> SM3 always produces a 256-bit hash value and there are no plans for
>> other length development, so there is no ambiguity in the name of
>> sm3.
>
> For the TPM we're following the TPM Library specification
>
> https://trustedcomputinggroup.org/resource/tpm-library-specification/
>
> Which is very clear: the algorithm name is TPM_ALG_SM3_256
>
> We're using sm3 as our exposed name because that's what linux crypto
> uses, so there should be no problem in what the end user sees, but
> changing to non standard TPM definitions is only going to cause
> confusion at the kernel level.
>
> James
>

Thanks for your attention. This is really tricky. I will contact
trustedcomputinggroup first and give some suggestions, It would be best
if a more standard algorithm name can be used from the source of the
specification.

I think the macro definition of the crypto directory can remove this
suffix first, that is, apply patch 1. What's your opinion?

Best regards,
Tianjia

2021-10-12 15:22:34

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256

On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> SM3 always produces a 256-bit hash value and there are no plans for
> other length development, so there is no ambiguity in the name of sm3.
>
> Signed-off-by: Tianjia Zhang <[email protected]>

This is not enough to make any changes because the commit message
does not describe what goes wrong if we keep it as it was.

/Jarkko

2021-10-14 09:46:55

by Tianjia Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256

Hi Jarkko,

On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
>> SM3 always produces a 256-bit hash value and there are no plans for
>> other length development, so there is no ambiguity in the name of sm3.
>>
>> Signed-off-by: Tianjia Zhang <[email protected]>
>
> This is not enough to make any changes because the commit message
> does not describe what goes wrong if we keep it as it was.
>
> /Jarkko
>

This did not cause an error, just to use a more standard algorithm name.
If it is possible to use the SM3 name instead of SM3_256 if it can be
specified from the source, it is of course better. I have contacted the
trustedcomputinggroup and have not yet received a reply.

Best regards,
Tianjia

2021-10-16 19:09:07

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256

On Thu, 2021-10-14 at 17:46 +0800, Tianjia Zhang wrote:
> Hi Jarkko,
>
> On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
> > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > > According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> > > SM3 always produces a 256-bit hash value and there are no plans for
> > > other length development, so there is no ambiguity in the name of sm3.
> > >
> > > Signed-off-by: Tianjia Zhang <[email protected]>
> >
> > This is not enough to make any changes because the commit message
> > does not describe what goes wrong if we keep it as it was.
> >
> > /Jarkko
> >
>
> This did not cause an error, just to use a more standard algorithm name.
> If it is possible to use the SM3 name instead of SM3_256 if it can be
> specified from the source, it is of course better. I have contacted the
> trustedcomputinggroup and have not yet received a reply.
>
> Best regards,
> Tianjia

Why don't you then create a patch set that fully removes SM3_256, if it
is incorrect?

This looks a bit half-baked patch set.

/Jarkko

2021-10-18 03:48:42

by Tianjia Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256

Hi Jarkko,

On 10/15/21 11:19 PM, Jarkko Sakkinen wrote:
> On Thu, 2021-10-14 at 17:46 +0800, Tianjia Zhang wrote:
>> Hi Jarkko,
>>
>> On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
>>> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>>>> According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
>>>> SM3 always produces a 256-bit hash value and there are no plans for
>>>> other length development, so there is no ambiguity in the name of sm3.
>>>>
>>>> Signed-off-by: Tianjia Zhang <[email protected]>
>>>
>>> This is not enough to make any changes because the commit message
>>> does not describe what goes wrong if we keep it as it was.
>>>
>>> /Jarkko
>>>
>>
>> This did not cause an error, just to use a more standard algorithm name.
>> If it is possible to use the SM3 name instead of SM3_256 if it can be
>> specified from the source, it is of course better. I have contacted the
>> trustedcomputinggroup and have not yet received a reply.
>>
>> Best regards,
>> Tianjia
>
> Why don't you then create a patch set that fully removes SM3_256, if it
> is incorrect?
>
> This looks a bit half-baked patch set.
>
> /Jarkko
>

This series of patch is a complete replacement. Patch 1 is a replacement
of the crypto subsystem, and patch 2 is a replacement of the tpm driver.

Best regards,
Tianjia

2021-10-18 12:49:15

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/2] tpm: use SM3 instead of SM3_256

On Mon, 2021-10-18 at 10:37 +0800, Tianjia Zhang wrote:
> Hi Jarkko,
>
> On 10/15/21 11:19 PM, Jarkko Sakkinen wrote:
> > On Thu, 2021-10-14 at 17:46 +0800, Tianjia Zhang wrote:
> > > Hi Jarkko,
> > >
> > > On 10/12/21 11:21 PM, Jarkko Sakkinen wrote:
> > > > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > > > > According to https://tools.ietf.org/id/draft-oscca-cfrg-sm3-01.html,
> > > > > SM3 always produces a 256-bit hash value and there are no plans for
> > > > > other length development, so there is no ambiguity in the name of sm3.
> > > > >
> > > > > Signed-off-by: Tianjia Zhang <[email protected]>
> > > >
> > > > This is not enough to make any changes because the commit message
> > > > does not describe what goes wrong if we keep it as it was.
> > > >
> > > > /Jarkko
> > > >
> > >
> > > This did not cause an error, just to use a more standard algorithm name.
> > > If it is possible to use the SM3 name instead of SM3_256 if it can be
> > > specified from the source, it is of course better. I have contacted the
> > > trustedcomputinggroup and have not yet received a reply.
> > >
> > > Best regards,
> > > Tianjia
> >
> > Why don't you then create a patch set that fully removes SM3_256, if it
> > is incorrect?
> >
> > This looks a bit half-baked patch set.
> >
> > /Jarkko
> >
>
> This series of patch is a complete replacement. Patch 1 is a replacement
> of the crypto subsystem, and patch 2 is a replacement of the tpm driver.
>
> Best regards,
> Tianjia

In which patch that symbol is removed?

/Jarkko

2021-10-18 13:06:44

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256

On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
[...]
> diff --git a/include/uapi/linux/hash_info.h
> b/include/uapi/linux/hash_info.h
> index 74a8609fcb4d..1355525dd4aa 100644
> --- a/include/uapi/linux/hash_info.h
> +++ b/include/uapi/linux/hash_info.h
> @@ -32,7 +32,7 @@ enum hash_algo {
> HASH_ALGO_TGR_128,
> HASH_ALGO_TGR_160,
> HASH_ALGO_TGR_192,
> - HASH_ALGO_SM3_256,
> + HASH_ALGO_SM3,
> HASH_ALGO_STREEBOG_256,
> HASH_ALGO_STREEBOG_512,
> HASH_ALGO__LAST

This is another one you can't do: all headers in UAPI are exports to
userspace and the definitions constitute an ABI. If you simply do a
rename, every userspace program that uses the current definition will
immediately break on compile. You could add HASH_ALGO_SM3, but you
can't remove HASH_ALGO_SM3_256

James


2021-10-18 13:28:58

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256

On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> [...]
> > diff --git a/include/uapi/linux/hash_info.h
> > b/include/uapi/linux/hash_info.h
> > index 74a8609fcb4d..1355525dd4aa 100644
> > --- a/include/uapi/linux/hash_info.h
> > +++ b/include/uapi/linux/hash_info.h
> > @@ -32,7 +32,7 @@ enum hash_algo {
> >         HASH_ALGO_TGR_128,
> >         HASH_ALGO_TGR_160,
> >         HASH_ALGO_TGR_192,
> > -       HASH_ALGO_SM3_256,
> > +       HASH_ALGO_SM3,
> >         HASH_ALGO_STREEBOG_256,
> >         HASH_ALGO_STREEBOG_512,
> >         HASH_ALGO__LAST
>
> This is another one you can't do: all headers in UAPI are exports to
> userspace and the definitions constitute an ABI.  If you simply do a
> rename, every userspace program that uses the current definition will
> immediately break on compile.  You could add HASH_ALGO_SM3, but you
> can't remove HASH_ALGO_SM3_256
>
> James

So: shouldn't then also the old symbol continue to work also
semantically?

/Jarkko

2021-10-18 13:37:43

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256

On Mon, 2021-10-18 at 16:27 +0300, Jarkko Sakkinen wrote:
> On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
> > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > [...]
> > > diff --git a/include/uapi/linux/hash_info.h
> > > b/include/uapi/linux/hash_info.h
> > > index 74a8609fcb4d..1355525dd4aa 100644
> > > --- a/include/uapi/linux/hash_info.h
> > > +++ b/include/uapi/linux/hash_info.h
> > > @@ -32,7 +32,7 @@ enum hash_algo {
> > > HASH_ALGO_TGR_128,
> > > HASH_ALGO_TGR_160,
> > > HASH_ALGO_TGR_192,
> > > - HASH_ALGO_SM3_256,
> > > + HASH_ALGO_SM3,
> > > HASH_ALGO_STREEBOG_256,
> > > HASH_ALGO_STREEBOG_512,
> > > HASH_ALGO__LAST
> >
> > This is another one you can't do: all headers in UAPI are exports
> > to userspace and the definitions constitute an ABI. If you simply
> > do a rename, every userspace program that uses the current
> > definition will immediately break on compile. You could add
> > HASH_ALGO_SM3, but you can't remove HASH_ALGO_SM3_256
> >
> > James
>
> So: shouldn't then also the old symbol continue to work also
> semantically?

Yes, that's the point: you can add a new definition ... in this case an
alias for the old one, but you can't remove a definition that's been
previously exported.

James


2021-10-18 13:59:22

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256

On Mon, 2021-10-18 at 09:32 -0400, James Bottomley wrote:
> On Mon, 2021-10-18 at 16:27 +0300, Jarkko Sakkinen wrote:
> > On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
> > > On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> > > [...]
> > > > diff --git a/include/uapi/linux/hash_info.h
> > > > b/include/uapi/linux/hash_info.h
> > > > index 74a8609fcb4d..1355525dd4aa 100644
> > > > --- a/include/uapi/linux/hash_info.h
> > > > +++ b/include/uapi/linux/hash_info.h
> > > > @@ -32,7 +32,7 @@ enum hash_algo {
> > > >         HASH_ALGO_TGR_128,
> > > >         HASH_ALGO_TGR_160,
> > > >         HASH_ALGO_TGR_192,
> > > > -       HASH_ALGO_SM3_256,
> > > > +       HASH_ALGO_SM3,
> > > >         HASH_ALGO_STREEBOG_256,
> > > >         HASH_ALGO_STREEBOG_512,
> > > >         HASH_ALGO__LAST
> > >
> > > This is another one you can't do: all headers in UAPI are exports
> > > to userspace and the definitions constitute an ABI.  If you simply
> > > do a rename, every userspace program that uses the current
> > > definition will immediately break on compile.  You could add
> > > HASH_ALGO_SM3, but you can't remove HASH_ALGO_SM3_256
> > >
> > > James
> >
> > So: shouldn't then also the old symbol continue to work also
> > semantically?
>
> Yes, that's the point: you can add a new definition ... in this case an
> alias for the old one, but you can't remove a definition that's been
> previously exported.

Thanks, this of course obvious :-) I forgot temporarily that crypto
has uapi interface. Tianjia, this patch set break production systems,
so no chance we would ever merge it in this form.

Why not just do this:

...
HASH_ALGO_SM3_256,
HASH_ALOG_SM3 = HASH_ALOG_SM_256,
...

There is not good reason to mod the implementation because both symbols
are kept.

/Jarkko

2021-10-19 09:36:27

by Tianjia Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256

Hi James,

On 10/18/21 9:05 PM, James Bottomley wrote:
> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
> [...]
>> diff --git a/include/uapi/linux/hash_info.h
>> b/include/uapi/linux/hash_info.h
>> index 74a8609fcb4d..1355525dd4aa 100644
>> --- a/include/uapi/linux/hash_info.h
>> +++ b/include/uapi/linux/hash_info.h
>> @@ -32,7 +32,7 @@ enum hash_algo {
>> HASH_ALGO_TGR_128,
>> HASH_ALGO_TGR_160,
>> HASH_ALGO_TGR_192,
>> - HASH_ALGO_SM3_256,
>> + HASH_ALGO_SM3,
>> HASH_ALGO_STREEBOG_256,
>> HASH_ALGO_STREEBOG_512,
>> HASH_ALGO__LAST
>
> This is another one you can't do: all headers in UAPI are exports to
> userspace and the definitions constitute an ABI. If you simply do a
> rename, every userspace program that uses the current definition will
> immediately break on compile. You could add HASH_ALGO_SM3, but you
> can't remove HASH_ALGO_SM3_256
>
> James
>

Correct, Thanks for pointing it out, redefining a macro is indeed a more
appropriate method.

Best regards,
Tianjia

2021-10-19 09:40:14

by Tianjia Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] crypto: use SM3 instead of SM3_256

Hi Jarkko,

On 10/18/21 9:41 PM, Jarkko Sakkinen wrote:
> On Mon, 2021-10-18 at 09:32 -0400, James Bottomley wrote:
>> On Mon, 2021-10-18 at 16:27 +0300, Jarkko Sakkinen wrote:
>>> On Mon, 2021-10-18 at 09:05 -0400, James Bottomley wrote:
>>>> On Sat, 2021-10-09 at 21:08 +0800, Tianjia Zhang wrote:
>>>> [...]
>>>>> diff --git a/include/uapi/linux/hash_info.h
>>>>> b/include/uapi/linux/hash_info.h
>>>>> index 74a8609fcb4d..1355525dd4aa 100644
>>>>> --- a/include/uapi/linux/hash_info.h
>>>>> +++ b/include/uapi/linux/hash_info.h
>>>>> @@ -32,7 +32,7 @@ enum hash_algo {
>>>>>         HASH_ALGO_TGR_128,
>>>>>         HASH_ALGO_TGR_160,
>>>>>         HASH_ALGO_TGR_192,
>>>>> -       HASH_ALGO_SM3_256,
>>>>> +       HASH_ALGO_SM3,
>>>>>         HASH_ALGO_STREEBOG_256,
>>>>>         HASH_ALGO_STREEBOG_512,
>>>>>         HASH_ALGO__LAST
>>>>
>>>> This is another one you can't do: all headers in UAPI are exports
>>>> to userspace and the definitions constitute an ABI.  If you simply
>>>> do a rename, every userspace program that uses the current
>>>> definition will immediately break on compile.  You could add
>>>> HASH_ALGO_SM3, but you can't remove HASH_ALGO_SM3_256
>>>>
>>>> James
>>>
>>> So: shouldn't then also the old symbol continue to work also
>>> semantically?
>>
>> Yes, that's the point: you can add a new definition ... in this case an
>> alias for the old one, but you can't remove a definition that's been
>> previously exported.
>
> Thanks, this of course obvious :-) I forgot temporarily that crypto
> has uapi interface. Tianjia, this patch set break production systems,
> so no chance we would ever merge it in this form.
>
> Why not just do this:
>
> ...
> HASH_ALGO_SM3_256,
> HASH_ALOG_SM3 = HASH_ALOG_SM_256,
> ...
>
> There is not good reason to mod the implementation because both symbols
> are kept.
>
> /Jarkko
>

Very good suggestion, I will do this in the next version patch. Maybe
this is more appropriate:

HASH_ALGO_SM3,
HASH_ALGO_SM3_256 = HASH_ALGO_SM3,

Best regards,
Tianjia