linux-kernel - Re: [RFC PATCH 1/2] x86: Fix rbp saving in pt

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: <20110106171231.GD2308@nowhere>
Date:	Thu, 6 Jan 2011 18:12:33 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Jan Beulich <JBeulich@...ell.com>
Cc:	"H. Peter Anvin" <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>,
	Stephane Eranian <eranian@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Soeren Sandmann Pedersen <sandmann@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/2] x86: Fix rbp saving in pt_regs on irq entry

On Thu, Jan 06, 2011 at 04:58:54PM +0000, Jan Beulich wrote:
> >>> On 06.01.11 at 17:54, Frederic Weisbecker <fweisbec@...il.com> wrote:
> > On Thu, Jan 06, 2011 at 04:39:39PM +0000, Jan Beulich wrote:
> >> >>> On 06.01.11 at 17:22, Frederic Weisbecker <fweisbec@...il.com> wrote:
> >> > On Thu, Jan 06, 2011 at 04:10:55PM +0000, Jan Beulich wrote:
> >> >> >>> On 06.01.11 at 16:45, Frederic Weisbecker <fweisbec@...il.com> wrote:
> >> >> > Before we had:
> >> >> > 
> >> >> > 
> >> >> > 	leaveq
> >> >> > 
> >> >> > 	CFI_RESTORE             rbp
> >> >> > 	CFI_DEF_CFA_REGISTER    rsp
> >> >> > 	CFI_ADJUST_CFA_OFFSET   -8
> >> >> > 
> >> >> > So CFI_RESTORE means rbp has now the value of the base frame of
> >> >> > the calling frame (the base frame pointer of the interrupted proc) ?
> >> >> 
> >> >> No - all it means is that %rbp now has its original (caller or
> >> >> interrupted procedure) value again (i.e. an unwinder should not
> >> >> try to read it from the stack [or other previously recorded
> >> >> location] anymore).
> >> >> 
> >> >> > And what follows means that rsp-8 points to the return address?
> >> >> 
> >> >> No - .cfi_def_cfa_register says which register serves as the frame
> >> >> pointer, and .cfi_adjust_cfa_offset says to adjust the offset from
> >> >> the frame pointer to the top [or bottom] of frame. At any time
> >> >> 
> >> >> 	CFA = cfa_register + cfa_offset
> >> >> 
> >> >> with CFA being what all locations on the stack are expressed
> >> >> relative to.
> >> > 
> >> > Ok.
> >> > 
> >> > So here rsp points to pt_regs::r11
> >> > 
> >> > I don't understand why locations relative to the stack must be
> >> > expressed here by taking rsp - 8 as a base.
> >> 
> >> Nothing says rsp-8. The annotations merely say to set the base
> >> register to rsp and to *adjust* the offset by -8 (after all, that's
> >> what the leaveq instruction does).
> > 
> > Ah! So CFA acts like a virtual frame base pointer right?
> 
> Correct.

Ah great. I was starting to prepare for the case you come to stab me :)

So what do you think about that:

        leaveq

        CFI_RESTORE             rbp
        CFI_DEF_CFA_REGISTER    rsp
        CFI_ADJUST_CFA_OFFSET   -8

       /* we did not save rbx, restore only from ARGOFFSET */
       addq $8, %rsp
       CFI_ADJUST_CFA_OFFSET   -16


Does that look correct to you? We increased rsp to start recovering
the regs from the right place, but the frame pointer of the current
proc must stay what it was.

Now I don't understand how this is all useful as this is not a normal
proc but an interruption. We can't get back the return address from
the CFA. Or am I missing something?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/