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

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: <ed6bfa1a-7e82-8273-ee7f-0e8097397724@suse.com>
Date:   Wed, 19 Apr 2023 18:03:53 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Borislav Petkov <bp@...en8.de>,
        Josh Poimboeuf <jpoimboe@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>, x86-ml <x86@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] x86/urgent for v6.3-rc7

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

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)