linux-kernel - Re: [PATCH v10 04/11] x86/entry/64: Adapt assembly for PIE support

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]

Date:   Fri, 6 Dec 2019 11:26:49 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Thomas Garnier <thgarnie@...omium.org>
Cc:     Kernel Hardening <kernel-hardening@...ts.openwall.com>,
        Kristen Carlson Accardi <kristen@...ux.intel.com>,
        Kees Cook <keescook@...omium.org>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        the arch/x86 maintainers <x86@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v10 04/11] x86/entry/64: Adapt assembly for PIE support

On Thu, Dec 05, 2019 at 09:01:50AM -0800, Thomas Garnier wrote:
> On Thu, Dec 5, 2019 at 1:04 AM Peter Zijlstra <peterz@...radead.org> wrote:
> > On Wed, Dec 04, 2019 at 04:09:41PM -0800, Thomas Garnier wrote:
> >
> > > @@ -1625,7 +1627,11 @@ first_nmi:
> > >       addq    $8, (%rsp)      /* Fix up RSP */
> > >       pushfq                  /* RFLAGS */
> > >       pushq   $__KERNEL_CS    /* CS */
> > > -     pushq   $1f             /* RIP */
> > > +     pushq   $0              /* Future return address */
> >
> > We're building an IRET frame, the IRET frame does not have a 'future
> > return address' field.
> 
> I assumed that's the target RIP after iretq.

It is. But it's still the (R)IP field of the IRET frame. Calling it
anything else is just confusing. The frame is 5 words: SS, (R)SP, (R)FLAGS,
CS, (R)IP.

> > > +     pushq   %rdx            /* Save RAX */
> > > +     leaq    1f(%rip), %rdx  /* RIP */
> >
> > nonsensical comment
> 
> That was the same comment from the push $1f that I changed.

Yes, but there it made sense since the PUSH actually created that field
of the frame, here it is nonsensical. What this instruction does is put
the address of the '1f' label into RDX, which is then stuck into the
(R)IP field on the next instruction.

> > > +     movq    %rdx, 8(%rsp)   /* Put 1f on return address */
> > > +     popq    %rdx            /* Restore RAX */