Question about -rc and stable patches

Question about -rc and stable patches

[Steven Rostedt]

Hi Linus and Greg,

Recently I have been sent patches that fix a bug in the enabling of
ftrace function tracer, where if something goes wrong with the
enabling (or disabling), it can cause a crash of the computer.

Now, my question is, the bug requires something to go wrong at a point
where I've never seen it go wrong before. But if you inject a problem
(code that makes it fail on the 1000th iteration), sure enough, the
recovery is not correct and the machine crashes.

Is this fix something that I should push to -rc as well as to stable?
Or should I just add this to my 3.15 queue, as I have never heard of
someone triggering it. The only report is from someone that was testing
the failure cases.

Thanks,

-- Steve

Re: Question about -rc and stable patches

[Steven Rostedt]

On Mon, 3 Mar 2014 14:41:04 -0800
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Mon, Mar 03, 2014 at 05:28:06PM -0500, Steven Rostedt wrote:
> > Hi Linus and Greg,
> > 
> > Recently I have been sent patches that fix a bug in the enabling of
> > ftrace function tracer, where if something goes wrong with the
> > enabling (or disabling), it can cause a crash of the computer.
> > 
> > Now, my question is, the bug requires something to go wrong at a point
> > where I've never seen it go wrong before. But if you inject a problem
> > (code that makes it fail on the 1000th iteration), sure enough, the
> > recovery is not correct and the machine crashes.
> 
> What exactly do you mean by "inject a problem"?  What kind of
> permissions does that require to do?

Modify kernel source ;-)  Basically, I added this patch to the kernel,
compiled it, enabled function tracing, and it crashed:

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8cabf63..9f354e5 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -586,6 +586,9 @@ void ftrace_replace_code(int enable)
 	for_ftrace_rec_iter(iter) {
 		rec = ftrace_rec_iter_record(iter);
 
+		if (count == 1000)
+			ret = -EPERM;
+		else
 		ret = add_breakpoints(rec, enable);
 		if (ret)
 			goto remove_breakpoints;


Not something that can be done from normal operations.

-- Steve

Re: Question about -rc and stable patches

[Steven Rostedt]

On Mon, 3 Mar 2014 17:45:32 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:


> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 8cabf63..9f354e5 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -586,6 +586,9 @@ void ftrace_replace_code(int enable)
>  	for_ftrace_rec_iter(iter) {
>  		rec = ftrace_rec_iter_record(iter);
>  
> +		if (count == 1000)
> +			ret = -EPERM;
> +		else
>  		ret = add_breakpoints(rec, enable);
>  		if (ret)
>  			goto remove_breakpoints;
> 
> 
> Not something that can be done from normal operations.
> 

I'll also add that one reason that this hasn't been an issue before, is
that the code that converts calls to mcount to nops, is done at kernel
boot up. If the above code were to fail, it is highly likely that the
code that converted mcount to nops would fail too. And if that happens,
it completely disables function tracing, preventing this code from
running.

The code that converts mcount to nops, handles the above error properly.

-- Steve