Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 7 Jun 2019 19:41:39 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Nadav Amit <namit@vmware.com>
Cc:     the arch/x86 maintainers <x86@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        Andy Lutomirski <luto@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Jason Baron <jbaron@akamai.com>, Jiri Kosina <jkosina@suse.cz>,
        David Laight <David.Laight@ACULAB.COM>,
        Borislav Petkov <bp@alien8.de>,
        Julia Cartwright <julia@ni.com>, Jessica Yu <jeyu@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Edward Cree <ecree@solarflare.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH 11/15] static_call: Add inline static call infrastructure
Message-ID: <20190607174139.GL3436@hirez.programming.kicks-ass.net>
References: <20190605130753.327195108@infradead.org>
 <20190605131945.193241464@infradead.org>
 <37CFAEC1-6D36-4A6D-8C44-F85FCFA053AA@vmware.com>
 <20190607083756.GW3419@hirez.programming.kicks-ass.net>
 <AF3846D0-01F0-4A42-AEB6-09B0902A659C@vmware.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <AF3846D0-01F0-4A42-AEB6-09B0902A659C@vmware.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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 :/