linux-kernel - Re: [PATCH] s390/crypto: make cca_public_sec and cca_token

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170809055257.GA4583@osiris>
Date:   Wed, 9 Aug 2017 07:52:58 +0200
From:   Heiko Carstens <heiko.carstens@...ibm.com>
To:     Bhumika Goyal <bhumirks@...il.com>
Cc:     julia.lawall@...6.fr, freude@...ibm.com, schwidefsky@...ibm.com,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const

On Sun, Aug 06, 2017 at 11:22:27AM +0530, Bhumika Goyal wrote:
> Declare cca_public_sec and cca_token_hdr structures as const as they are
> only used during copy operations.
> 
> Signed-off-by: Bhumika Goyal <bhumirks@...il.com>
> ---
>  drivers/s390/crypto/zcrypt_cca_key.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/crypto/zcrypt_cca_key.h b/drivers/s390/crypto/zcrypt_cca_key.h
> index 12cff62..7b78ba6 100644
> --- a/drivers/s390/crypto/zcrypt_cca_key.h
> +++ b/drivers/s390/crypto/zcrypt_cca_key.h
> @@ -116,10 +116,10 @@ struct cca_pvt_ext_CRT_sec {
>   */
>  static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
>  {
> -	static struct cca_token_hdr static_pub_hdr = {
> +	static const struct cca_token_hdr static_pub_hdr = {
>  		.token_identifier	=  0x1E,
>  	};
> -	static struct cca_public_sec static_pub_sec = {
> +	static const struct cca_public_sec static_pub_sec = {
>  		.section_identifier	=  0x04,
>  	};
>  	struct {
> @@ -176,7 +176,7 @@ static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
>   */
>  static inline int zcrypt_type6_crt_key(struct ica_rsa_modexpo_crt *crt, void *p)
>  {
> -	static struct cca_public_sec static_cca_pub_sec = {
> +	static const struct cca_public_sec static_cca_pub_sec = {
>  		.section_identifier = 4,
>  		.section_length = 0x000f,
>  		.exponent_len = 0x0003,

If you look at the code, it seems to be questionable why these structures
are declared static and not simply on the stack. In the first case we are
talking of an assignment of two single bytes two a structure that has
already been zeroed.
Besides this, the two functions in the header file are very large and
shouldn't be unconditionally inlined anyway. This needs to be cleaned
up. So I'm not at all in favor of a patch that just adds a const keyword in
addition.