linux-kernel - Re: [PATCH v3 8/7] TESTING_ONLY x86/entry: reduce static footprint of idtentry

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 13 Feb 2018 13:41:06 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Dominik Brodowski <linux@...inikbrodowski.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andi Kleen <ak@...ux.intel.com>,
        Andrew Lutomirski <luto@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Brian Gerst <brgerst@...il.com>, Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v3 8/7] TESTING_ONLY x86/entry: reduce static footprint
 of idtentry


* Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Mon, Feb 12, 2018 at 1:37 AM, Ingo Molnar <mingo@...nel.org> wrote:
> >
> > Ok, so this does not look _that_ complicated, and the .text savings are
> > significant:
> 
> Honestly, I think we should do it. 3kB of assembly code is noticeable.
> 
> Also, that patch actually allows more cleanups and simplifications.
> Look at the "interrupt" macro, which is used by 'apicinterrupt3', and
> has a number of uses that way.
> 
> That code could be unified a lot, right now it does:
> 
>         testb   $3, CS-ORIG_RAX(%rsp)
>         jz      1f
>         SWAPGS
>         call    switch_to_thread_stack
> 1:
> 
>         ALLOC_PT_GPREGS_ON_STACK
>         SAVE_C_REGS
>         SAVE_EXTRA_REGS
>         ENCODE_FRAME_POINTER
> 
>         testb   $3, CS(%rsp)
>         jz      1f
> 
>         /*
>          * IRQ from user mode.
>          *
>          * We need to tell lockdep that IRQs are off.  We can't do this until
>          * we fix gsbase, and we should do it before enter_from_user_mode
>          * (which can take locks).  Since TRACE_IRQS_OFF idempotent,
>          * the simplest way to handle it is to just call it twice if
>          * we enter from user mode.  There's no reason to optimize this since
>          * TRACE_IRQS_OFF is a no-op if lockdep is off.
>          */
>         TRACE_IRQS_OFF
> 
>         CALL_enter_from_user_mode
> 
> 1:
>         ENTER_IRQ_STACK old_rsp=%rdi
>         /* We entered an interrupt context - irqs are off: */
>         TRACE_IRQS_OFF
> 
> and *all* of that could be in a helper function rather than be
> duplicated. and the apicinterrupt3 macro should end up just expanding
> to
> 
>         callq helper
>         pushq $~(\num)
>         callq \fn
>         jmp ret_from_intr
> 
> instead of expanding to all that code.
> 
> But that would require that same "save_ret" logic.
> 
> So it's not just the idtentry cases that can use this trick.
> 
> I admit that the trick isn't pretty, but it's not *horribly* ugly either.

Ok, agreed!

Dominik, could you please do this on top latest tip:master? I have applied all the 
other patches. Note that patch #8 now conflicts with recent annotation fixes.

Thanks,

	Ingo