linux-kernel - Re: [RFC] crypto/sm4: Fix objtool/libelf warning

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: <6759f51a-4f35-5a68-4934-24947f07e85b@linux.alibaba.com>
Date:   Fri, 15 Oct 2021 11:12:02 +0800
From:   Tianjia Zhang <tianjia.zhang@...ux.alibaba.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     herbert@...dor.apana.org.au, linux-kernel@...r.kernel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>, x86@...nel.org
Subject: Re: [RFC] crypto/sm4: Fix objtool/libelf warning

Hi Peter,

On 10/14/21 10:56 PM, Peter Zijlstra wrote:
> On Thu, Oct 14, 2021 at 06:29:55PM +0800, Tianjia Zhang wrote:
>> Hi Peter,
>>
>> On 10/8/21 4:22 AM, Peter Zijlstra wrote:
>>> Hi,
>>>
>>> objtool is yielding the obscure libelf warning:
>>>
>>>     vmlinux.o: warning: objtool: elf_update: invalid section entry size
>>>
>>> Which I tracked down to section:
>>>
>>>     [3023] .rodata.cst164    PROGBITS        0000000000000000 1ab501e0 000154 a4  AM  0   0 16
>>>
>>> Which has a section size of 0x154 (340) and an entry size of 0xa4 (164).
>>> An obvious mis-match.
>>>
>>>   From there, git-grep quickly yields:
>>>
>>>     arch/x86/crypto/sm4-aesni-avx-asm_64.S:.section .rodata.cst164, "aM", @progbits, 164
>>>     arch/x86/crypto/sm4-aesni-avx2-asm_64.S:.section        .rodata.cst164, "aM", @progbits, 164
>>>
>>> So those files create this .rodata section with an explicit entry size,
>>> but then don't respect it themselves. Removing the entry size makes the
>>> warning go away, but I can't tell if that's right or not, given there is
>>> zero clue as to why that entry size was specified to begin with.
>>>
>>> Please explain...
>>>
>>> ---
>>> diff --git a/arch/x86/crypto/sm4-aesni-avx-asm_64.S b/arch/x86/crypto/sm4-aesni-avx-asm_64.S
>>> index 18d2f5199194..d089cccf4db7 100644
>>> --- a/arch/x86/crypto/sm4-aesni-avx-asm_64.S
>>> +++ b/arch/x86/crypto/sm4-aesni-avx-asm_64.S
>>> @@ -78,7 +78,7 @@
>>>    	vpxor tmp0, x, x;
>>> -.section	.rodata.cst164, "aM", @progbits, 164
>>> +.section	.rodata.cst164, "aM", @progbits
>>>    .align 16
>>>    /*
>>> diff --git a/arch/x86/crypto/sm4-aesni-avx2-asm_64.S b/arch/x86/crypto/sm4-aesni-avx2-asm_64.S
>>> index d2ffd7f76ee2..a0f7541c2246 100644
>>> --- a/arch/x86/crypto/sm4-aesni-avx2-asm_64.S
>>> +++ b/arch/x86/crypto/sm4-aesni-avx2-asm_64.S
>>> @@ -93,7 +93,7 @@
>>>    	vpxor tmp0, x, x;
>>> -.section	.rodata.cst164, "aM", @progbits, 164
>>> +.section	.rodata.cst164, "aM", @progbits
>>>    .align 16
>>>    /*
>>>
>>
>> Thanks for pointing it out, We have also reproduced this error. If the M
>> flag is specified, the entry_size argument is required.
> 
> Correct.
> 
>> We also need to
>> consider the clang compiler. This requires a more thorough method to fix it.
>> I will post another patch later.
> 
> If the purpose is to share the whole section, such that there is only a
> single copy of those tables between the two sm4 implementations, then
> you need to set the entry size to the total size of the section.
> 
> Otoh, almost every entry (with exception of the very last one) seems to
> be 16 bytes, so you might just get away with setting the entry size to
> 16.
> 
> Given this is only a very small data table, why the need to share? Any
> one machine will only use a single one of these implementations at any
> one time.
> 

The main purpose is not to share the whole section, but to prevent clang 
from causing errors. It seems that the clang compiler has stricter 
restrictions on this entry_size.

Best regards,
Tianjia