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] [day] [month] [year] [list]

Message-ID: <CAOH+1jEJJsghh38kBJq_ix0-BrBcmSaFHio3hMye=sS=QS0K=w@mail.gmail.com>
Date:   Wed, 9 Aug 2017 13:09:53 +0530
From:   Bhumika Goyal <bhumirks@...il.com>
To:     Heiko Carstens <heiko.carstens@...ibm.com>
Cc:     Julia Lawall <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 Wed, Aug 9, 2017 at 11:22 AM, Heiko Carstens
<heiko.carstens@...ibm.com> wrote:
> 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.
>

Thanks for the explanation. I think that it would be better to drop the patch.

Thanks,
Bhumika