Re: [PATCH 2/4] ftrace/x86: Do not crash when reading wrong ftrace func

[Petr Mladek]

On Mon 2016-06-27 10:48:58, Steven Rostedt wrote:
> On Mon, 27 Jun 2016 15:54:35 +0200
> Petr Mladek <pmladek@xxxxxxxx> wrote:
> 
> > Ftrace modifies the code on many locations. It is paranoid
> > and avoid a kernel crash using probe_kernel_read() and
> > probe_kernel_write(). The only exception is update_ftrace_func()
> > where where we read the old code using memcpy().
> > 
> > It is true that this function is used only to modify well
> > defined functions that are part of the ftrace API. But
> > it might still make sense to be paranoid and be consistent
> > with the writing side.
> 
> I'm not so sure I'm too hot on this patch. I left it with the memcpy()
> because it was a well known location. If this is wrong, then we should
> crash the kernel.

I had a suspicion that you had had a reason for using memcpy(). This
is why I put it into a separate patch ;-)

Well, I still would feel better if the kernel survive. An error here
means that there is a bug in the core ftrace infrastructure but
it does not need to be fatal for the running system.


> I'm not totally against it. But a comment should be added stating
> something like:
> 
> 	/*
> 	 * ip points to the ftrace infrastructure. If this fails,
> 	 * then something is totally messed up.
> 	 */
> 
> and perhaps even add a WARN_ON() here too.

Yup, WARN_ON() seems appropriate. I like it. Please,
find the updated patch below.