Re: [GIT PULL] x86/urgent for v6.3-rc7

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2118 bytes --]

On 19.04.23 17:59, Borislav Petkov wrote:
> On Mon, Apr 17, 2023 at 06:24:35PM -0700, Josh Poimboeuf wrote:
>> I implemented it in the patch below.
> 
> Ha, amazing! Thanks man. :-)
> 
>> It was pretty straightforward
>> since this case always(?) involves two relocations within a single
>> instruction, like:
>>
>>   3d9:	48 c7 05 00 00 00 00 00 00 00 00 	movq   $0x0,0x0(%rip)        # 3e4 <x86_wallclock_init+0xa4>	3dc: R_X86_64_PC32	x86_platform+0x8
>> 	3e0: R_X86_64_32S	.init.text+0x280
>>
>> So objtool warns if the instruction's first relocation is non-init and
>> the second one is init.
> 
> Sounds plausible to me.
> 
>> There are 9 "disallowed store" warnings.  There would have been more,
>> but I added per-symbol rate-limiting to keep the total number of
>> warnings down.  For example, instead of the x86_wallclock_init() issue
>> discovered above, it shows a similar warning from elsewhere:
>>
>>    vmlinux.o: warning: objtool: xen_init_apic+0xab: disallowed store of init symbol xen_apic_check to non-init data x86_platform
> 
> Obviously a good catch.
> 
> However, the only call site is in apic_intr_mode_init() which itself is
> __init. So yeah, strictly speaking nothing wrong.
> 
> *IF* something calls it later, when __init is gone, then boom.
> 
> Dunno, maybe
> 
> a) track call sites too but maybe expensive and too much
> 
> or
> 
> b) make those warnings debug messages in case someone wants to run objtool
> in debug mode, feels really bored and wants to fix potential issues.
> 
> Or someone has a better idea.
> 
>> Anyway, this is kind of experimental.  If any of these warnings don't
>> turn out to be useful I could drop some or all them.
> 
> Right, I can certainly see potential and as said, since we're already
> doing objtool massaging of object files, thought this should be pretty
> easy to do. As you've shown. ;-)

What about splitting the function vectors into __init and non-init parts?

This would solve above problem automatically, as a non-init function couldn't
deref the __init part of x86_platform.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]