linux-kernel - Re: [patch 01/15] x86/irq: Convey vector as argument and not in ptregs

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: <CAMzpN2j7EHZ2bKg9SZ2Ri-qsmEoknAAJO6O5yoLn-fY8_h1B2A@mail.gmail.com>
Date:   Wed, 26 Feb 2020 16:54:33 -0500
From:   Brian Gerst <brgerst@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Juergen Gross <jgross@...e.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [patch 01/15] x86/irq: Convey vector as argument and not in ptregs

On Wed, Feb 26, 2020 at 3:13 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> Brian Gerst <brgerst@...il.com> writes:
>
> > On Tue, Feb 25, 2020 at 6:26 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> >>
> >> Device interrupts which go through do_IRQ() or the spurious interrupt
> >> handler have their separate entry code on 64 bit for no good reason.
> >>
> >> Both 32 and 64 bit transport the vector number through ORIG_[RE]AX in
> >> pt_regs. Further the vector number is forced to fit into an u8 and is
> >> complemented and offset by 0x80 for historical reasons.
> >
> > The reason for the 0x80 offset is so that the push instruction only
> > takes two bytes.  This allows each entry stub to be packed into a
> > fixed 8 bytes.  idt_setup_apic_and_irq_gates() assumes this 8-byte
> > fixed length for the stubs, so now every odd vector after 0x80 is
> > broken.
> >
> >      508:       6a 7f                   pushq  $0x7f
> >      50a:       e9 f1 08 00 00          jmpq   e00 <common_interrupt>
> >      50f:       90                      nop
> >      510:       68 80 00 00 00          pushq  $0x80
> >      515:       e9 e6 08 00 00          jmpq   e00 <common_interrupt>
> >      51a:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
> >      520:       68 81 00 00 00          pushq  $0x81
> >      525:       e9 d6 08 00 00          jmpq   e00 <common_interrupt>
> >      52a:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
> >
> > The 0x81 vector should start at 0x518, not 0x520.
>
> Bah, I somehow missed that big fat comment explaining it. :)
>
> Thanks for catching it. So my testing just has been lucky to not hit one
> of those.
>
> Now the question is whether we care about the packed stubs or just make
> them larger by using alignment to get rid of this silly +0x80 and
> ~vector fixup later on. The straight forward thing clearly has its charm
> and I doubt it matters in measurable ways.

I think we can get rid of the inversion.  That was done so orig_ax had
a negative number (signifying it's not a syscall), but if you replace
it with -1 that isn't necessary.  A simple -0x80 offset should be
sufficient.

I think it's a worthy optimization to keep.  There are 240 of these
stubs, so increasing the allocation to 16 bytes would add 1920 bytes
to the kernel text.

--
Brian Gerst