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-next>] [day] [month] [year] [list]

Message-Id: <4D25EB4B020000780002ABF7@vpn.id2.novell.com>
Date:	Thu, 06 Jan 2011 15:18:19 +0000
From:	"Jan Beulich" <JBeulich@...ell.com>
To:	"Frederic Weisbecker" <fweisbec@...il.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 06.01.11 at 15:51, Frederic Weisbecker <fweisbec@...il.com> wrote:
> From the x86_64 low level interrupt handlers, the frame pointer is
> saved in pt_regs in the wrong place, in the offset of rbx.
> 
> rbx is not part of the irq partial saved registers, so it's not
> critical, but later code that uses get_irq_regs() to get the
> interrupted frame pointer instead get a stale rbx value, causing
> unwinding code to fail miserably.

Code using get_irq_regs() can't rely on the not explicitly
saved registers' fields of pt_regs anyway; you now fix this
for %rbp, but someone else might look at a different
register and want that to be saved too. You just shouldn't,
and the fact that %rbp happens to be saved at all shouldn't
be taken to mean you can access it via the provided
pt_regs pointer. This saving of %rbp could go away or be
done in a different way at any point.

> @@ -808,6 +813,8 @@ ret_from_intr:
>  	TRACE_IRQS_OFF
>  	decl PER_CPU_VAR(irq_count)
>  	leaveq
> +	/* we did not save rbx, restore only from ARGOFFSET */
> +	addq $8, %rsp
>  	CFI_RESTORE		rbp
>  	CFI_DEF_CFA_REGISTER	rsp
>  	CFI_ADJUST_CFA_OFFSET	-8

*If* the patch was to be taken anyway, I would strongly suggest
getting this mis-insertion fixed first: CFI annotations belong to the
immediately preceding instruction, and hence you must not insert
new instructions between an existing one and its annotation.

Furthermore, an adjustment to %rsp (when it serves as the frame
pointer as is the case here, though would be better visible if the
added instruction was at the right place) needs to be annotated
itself.

Jan

--
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/