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:   Wed, 31 Oct 2018 14:53:20 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Zhenzhong Duan <zhenzhong.duan@oracle.com>
Cc:     Linux-Kernel <linux-kernel@vger.kernel.org>, mingo@redhat.com,
        konrad.wilk@oracle.com, dwmw@amazon.co.uk, tglx@linutronix.de,
        Srinivas REDDY Eeda <srinivas.eeda@oracle.com>, bp@suse.de,
        hpa@zytor.com
Subject: Re: [PATCH 3/3] kprobes/x86: Simplify indirect-jump check in
 retpoline
Message-ID: <20181031135320.GC13237@hirez.programming.kicks-ass.net>
References: <cc8f9981-f327-4238-8378-262579584db4@default>
 <20181030083650.GB1459@hirez.programming.kicks-ass.net>
 <849ae148-85cd-5f46-d98b-b827cc9c605c@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <849ae148-85cd-5f46-d98b-b827cc9c605c@oracle.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Oct 31, 2018 at 02:01:20PM +0800, Zhenzhong Duan wrote:
> On 2018/10/30 16:36, Peter Zijlstra wrote:
> > On Mon, Oct 29, 2018 at 11:55:06PM -0700, Zhenzhong Duan wrote:
> > > Since CONFIG_RETPOLINE hard depends on compiler support now, so
> > > replacing indirect-jump check with the range check is safe in that case.
> > 
> > Can we put kprobes on module init text before we run alternatives on it?
> 
> Forgive me I doesn't understand your question. Do you mean this patch impact
> kprobes on module init text?

In that case we would still see the indirect paravirt calls for example,
and we'd still need that cascade you took out.

Now, I'm not at all sure we're able to use kprobes at those times, so it
might be a non-issue.

> > > @@ -240,20 +242,16 @@ static int insn_jump_into_range(struct insn *insn, unsigned long start, int len)
> > >   static int insn_is_indirect_jump(struct insn *insn)
> > >   {
> > > -	int ret = __insn_is_indirect_jump(insn);
> > > +	int ret;
> > >   #ifdef CONFIG_RETPOLINE
> > > -	/*
> > > -	 * Jump to x86_indirect_thunk_* is treated as an indirect jump.
> > > -	 * Note that even with CONFIG_RETPOLINE=y, the kernel compiled with
> > > -	 * older gcc may use indirect jump. So we add this check instead of
> > > -	 * replace indirect-jump check.
> > > -	 */
> > > -	if (!ret)
> > > +	/* Jump to x86_indirect_thunk_* is treated as an indirect jump. */
> > >   		ret = insn_jump_into_range(insn,
> > >   				(unsigned long)__indirect_thunk_start,
> > >   				(unsigned long)__indirect_thunk_end -
> > >   				(unsigned long)__indirect_thunk_start);
> > > +#else
> > > +		ret = __insn_is_indirect_jump(insn);
> > >   #endif
> > >   	return ret;
> > >   }
> > 
> > The resulting code is indented wrong.
> > 
> 
> Oh, yes. Thanks for point out.
> 
> Zhenzhong