linux-kernel - Re: [v5][PATCH] livepatch/ppc: Enable livepatching on powerpc

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: <56DF9B5B.6070300@gmail.com>
Date:	Wed, 9 Mar 2016 14:41:15 +1100
From:	Balbir Singh <bsingharora@...il.com>
To:	Torsten Duwe <duwe@....de>
Cc:	linuxppc-dev@...abs.org, pmladek@...e.com, jeyu@...hat.com,
	jkosina@...e.cz, jikos@...nel.org, linux-kernel@...r.kernel.org,
	rostedt@...dmis.org, kamalesh@...ux.vnet.ibm.com,
	live-patching@...r.kernel.org, mbenes@...e.cz
Subject: Re: [v5][PATCH] livepatch/ppc: Enable livepatching on powerpc



On 09/03/16 02:34, Torsten Duwe wrote:
> On Wed, Mar 09, 2016 at 12:52:03AM +1100, Balbir Singh wrote:
>> On 08/03/16 21:45, Torsten Duwe wrote:
>>> To be fair, my last mail still was not 100% correct, but the conclusion
> Wrote a correction to the correction. It should be clear now. Please nag me
> if it isn't clear why klp_return_helper and its stack frame is needed.
>
>>> that the mini frame is not needed at all is invalid. Please leave it as it
>>> was, I'm working on a test / demonstrator for how to handle these.
>> Why, the magic will be in the patched function? Please share the test/demonstrator
> Here it comes...
>
>>> NAKed-by: Torsten Duwe <duwe@...e.de>
>> Why? For using CR+4 or removing the frame? Or you believe there is a better way to
>> handle this that work, IOW what is broken?
> The stack frame removal. You're risking a memory access or jump into nirvana
> or and endless loop.
>
> klp_return_helper will do the right thing, and functions like e.g. printk
> I would live patch like this (untested :-) :
>
> ------------------------8<----------------------
> #include <stdarg.h>
>
> /* compile using "-ffixed-r14"! */
> register unsigned long pass_TOC asm("r14");
>
> /*
>  * Function pre-prologue to pop the klp_return_helper
>  * mini stack frame. The saved r2 TOC value is read and
>  * passed in pass_TOC (r14), the original LR is passed
>  * in r0 and the LR itself. R12 is updated appropriately
>  * for local TOC recalculation.
>  */
> extern void caller(void) asm("printk");
> void caller(void)
> {
>   asm("ld %0,24(1)" : "=r" (pass_TOC));
>   asm("addi 1,1,32");
>   asm("addi 12,12,(real_printk-printk)@l");
>   asm("ld 0,16(1)\n\tmtlr 0");
>   asm("b real_printk");
> }
>
> extern int vprintk_default(const char *fmt, va_list args);
>
> extern int printk(const char *fmt, ...) asm("real_printk");
> int printk(const char *fmt, ...)
> {
>         va_list args;
>         int r;
>
>         va_start(args, fmt);
>
>         r = vprintk_default(fmt, args);
>
>         va_end(args);
>
> 	asm("mr 2,%0" : : "r" (pass_TOC));
>         return r;
> }
> ------------------------8<----------------------
> Signed-off-by: Torsten Duwe <duwe@...e.de>
>
> As you can see, the extra effort for args on the stack is limited.

Leaving it to the patch to do the right thing I think makes it more
complex and each livepatch hardware dependent to a large extent.
I find it hard to read the patch, let alone audit it and apply it or
worse create one

>
> 	Torsten
>
Balbir Singh.