linux-kernel - Re: [PATCH] s390: add support for DYNAMIC_FTRACE_WITH

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: <20140815115753.GA4215@osiris>
Date:	Fri, 15 Aug 2014 13:57:53 +0200
From:	Heiko Carstens <heiko.carstens@...ibm.com>
To:	Vojtech Pavlik <vojtech@...e.cz>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Jiri Kosina <jkosina@...e.cz>, Jiri Slaby <jslaby@...e.cz>,
	linux390@...ibm.com, linux-s390@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] s390: add support for DYNAMIC_FTRACE_WITH_REGS

On Tue, Jul 08, 2014 at 10:07:40AM +0200, Heiko Carstens wrote:
> On Thu, Jul 03, 2014 at 02:00:46PM +0200, Vojtech Pavlik wrote:
> > Add support for DYNAMIC_FTRACE_WITH_REGS to 64-bit and 31-bit s390
> > architectures. This is required for kGraft and kpatch to work on s390.
> > 
> > It's done by adding a _regs variant of ftrace_caller that preserves
> > registers and puts them on stack in a struct pt_regs layout and
> > allows modification of return address by changing the PSW (instruction
> > pointer) member od struct pt_regs. 
> >
> > Signed-off-by: Vojtech Pavlik <vojtech@...e.cz>
> > Reviewed-by: Jiri Kosina <jkosina@...e.cz>
> > Cc: Steven Rostedt <rostedt@...dmis.org>
> 
> So I assume you use the instruction_pointer() macro to access the
> return address then?
> 
> All of this seems a bit of a hack to me.. the natural place of the
> return address of a function would be register 14, and not the
> psw member of the pt_regs structure.
> 
> It's then also inconsistent to only save register r0-r13 to the
> gprs member.. well, you can't save r14, since what should
> happen if both r14 in the gprs member of pt_regs and in the psw
> part would have been changed?
> 
> Besides that a couple more comments below.

[...]

> Some objections: this code assumes that sizeof(struct pt_regs) does not
> change, which is not correct. So as soon as we touch struct pt_regs this
> code would be broken. Also the order of the members within struct pt_regs
> is not necessarily static (pt_regs is not ABI).

FWIW, this already happened with d3a73acbc26a4a81a01a35fd162973e53d0386f5
"s390: split TIF bits into CIF, PIF and TIF bits".

Anyway, since I didn't got any response from you during the last couple of
weeks, I changed the ftrace code so it should fit your needs.
I will send five patches in reply to this mail - patch 4 of 5 is the one
that implements the DYNAMIC_FTRACE_WITH_REGS functionality, how differently
to your patch, especially possible return address changing.

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