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

[Balbir Singh <bsingharora@gmail.com>]

On 09/03/16 03:02, Petr Mladek wrote:
> On Tue 2016-03-08 18:33:57, Balbir Singh wrote:
>> Changelog v5:
>> 	1. Removed the mini-stack frame created for klp_return_helper.
>> 	   As a result of the mini-stack frame, function with > 8
>> 	   arguments could not be patched
>> 	2. Removed camel casing in the comments
> I tested this patch and it fails when I call a patched printk()
> from a module.
>
> You might try it with the test patch below. It is a bit twisted
> because it calls the patched printk from livepatch_cmdline_proc_show()
> that it added by the same patch module. Please, look at
> livepatch_cmdline_proc_show(), it does:
>
> 	static int count;
>
> 	if (!count++)
> 		trace_printk("%s\n", "this has been live patched");
> 	else
> 		printk("%s\n", "this has been live patched");
>
>
> It means that calls only trace_printk() when called first time.
> It calls the patched printk when called second time.
>
>
> I have tested it the following way:
>
>
> # booted kernel with the changes below
> # applied the patch:
> $> modprobe livepatch-sample
>
> # trigger the pached printk()
> $>cat /sys/kernel/livepatch/livepatch_sample/enabled
> 1
>
> # look into both dmesg and trace buffer
> $> dmesg | tail -n 1
> [  727.537307] patch enabled: 1
> $> cat /sys/kernel/debug/tracing/trace | tail -n 1
>              cat-3588  [003] ....   727.537448: livepatch_printk: patch enabled: 1
>
> # trigger livepatch_cmdline_proc_show() 1st time
> c79:~ # cat /proc/cmdline 
> this has been live patched
>
> # the message appeared only in trace buffer
> $> dmesg | tail -n 1
> [  727.537307] patch enabled: 1
> c79:~ # cat /sys/kernel/debug/tracing/trace | tail -n 1
>              cat-3511  [000] ....    862.958383:	     livepatch_cmdline_proc_show: this has been live patched
>
>
> # trigger livepatch_cmdline_proc_show() 2nd time
> c79:~ # cat /proc/cmdline 
>
> !!! KABOOM !!!
>
> It is becaused it tried to call the patched printk()?
>
Yes, the situation is that we restored the r2 for the kernel (from ftrace_caller, it is now kernel_toc),
whereas the LR points to the module. The difference between r2 and r0 > 4GB.

Very good test case. Did it work with v4? I presume it did because we have enough space to save both

Thanks,
Balbir Singh