Re: [PATCH 11/15] static_call: Add inline static call infrastructure

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Jun 07, 2019 at 04:35:42PM +0000, Nadav Amit wrote:
> > On Jun 7, 2019, at 1:37 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Jun 06, 2019 at 10:24:17PM +0000, Nadav Amit wrote:

> >>> +		if (ret) {
> >>> +			WARN(1, "Failed to allocate memory for static calls");
> >>> +			static_call_del_module(mod);
> >> 
> >> If static_call_add_module() succeeded in changing some of the calls, but not
> >> all, I don’t think that static_call_del_module() will correctly undo
> >> static_call_add_module(). The code transformations, I think, will remain.
> > 
> > Hurm, jump_labels has the same problem.
> > 
> > I wonder why kernel/module.c:prepare_coming_module() doesn't propagate
> > the error from the notifier call. If it were to do that, I think we'll
> > abort the module load and any modifications get lost anyway.
> 
> This might be a security problem, since it can leave indirect branches,
> which are susceptible to Spectre v2, in the code.

It's a correctness problem too; for both jump_label and static_call,
since if we don't patch the call site, we also don't patch the
trampoline and who knows what random code it ends up running.

I'll go stare at the module code once my migrane goes again :/