linux-kernel - Re: [PATCH 6/6] powerpc/livepatch: Add live patching support on ppc64le

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: <20160324134221.GB29197@lst.de>
Date:	Thu, 24 Mar 2016 14:42:21 +0100
From:	Torsten Duwe <duwe@....de>
To:	Michael Ellerman <mpe@...erman.id.au>
Cc:	linuxppc-dev@...abs.org, bsingharora@...il.com,
	linux-kernel@...r.kernel.org, rostedt@...dmis.org,
	kamalesh@...ux.vnet.ibm.com, pmladek@...e.com, jeyu@...hat.com,
	jkosina@...e.cz, live-patching@...r.kernel.org, mbenes@...e.cz
Subject: Re: [PATCH 6/6] powerpc/livepatch: Add live patching support on
	ppc64le

On Thu, Mar 24, 2016 at 10:04:05PM +1100, Michael Ellerman wrote:
> +livepatch_handler:
> +	CURRENT_THREAD_INFO(r12, r1)
[...]
> +	/* Put ctr in r12 for global entry and branch there */
> +	mfctr	r12
> +	bctrl
            ^
I like this piece. No need to fiddle out the return helper address.

> +	/*
> +	 * Now we are returning from the patched function to the original
> +	 * caller A. We are free to use r0 and r12, and we can use r2 until we
> +	 * restore it.
> +	 */
> +
> +	CURRENT_THREAD_INFO(r12, r1)
> +
> +	/* Save stack pointer into r0 */
> +	mr	r0, r1
> +
> +	ld	r1, TI_livepatch_sp(r12)
> +
> +	/* Check stack marker hasn't been trashed */
> +	lis     r2,  STACK_END_MAGIC@h
> +	ori     r2,  r2, STACK_END_MAGIC@l
> +	ld	r12, -8(r1)
> +1:	tdne	r12, r2
> +	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0

This however worries me a bit. Sure, in the end, a stack overflow is
a stack overflow, and if all the information does not fit there,
there's little you can do.

But wouldn't it be better to kmalloc that area and realloc in
klp_arch_set_pc when it's full? Maybe along with a warning message?
That way a live patched kernel will not run into stack size problems
any earlier than an unpatched kernel would.
Just a thought.

Anyway, patch 5+6

Reviewed-by: Torsten Duwe <duwe@...e.de>

	Torsten