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

On Thu, 2016-03-24 at 14:42 +0100, Torsten Duwe wrote:

On Thu, Mar 24, 2016 at 10:04:05PM +1100, Michael Ellerman wrote:

quoted

+livepatch_handler:
+	CURRENT_THREAD_INFO(r12, r1)

[...]

quoted

+	/* 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.

Good.

quoted

+	/*
+	 * 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.

Yeah stack overflow in the kernel is very very fatal.

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?

You can't realloc in klp_arch_set_pc(), you might be patching sl*b and holding
one of its locks. You might also recurse.

We could allocate a larger buffer as a "klp stack" for each task when the first
live patch is installed, and for every task created afterward. But that
potentially significantly increases memory usage on live patched kernels :)

That way a live patched kernel will not run into stack size problems
any earlier than an unpatched kernel would.

Yeah that's true. I'm not sure what the best trade off is.

cheers