Re: [PATCH 00/33] Compile-time stack metadata validation

Josh Poimboeuf
2016-02-12T18:34:09Z

On Fri, Feb 12, 2016 at 12:32:06PM -0600, Josh Poimboeuf wrote:
> On Fri, Feb 12, 2016 at 06:10:37PM +0100, Peter Zijlstra wrote:
> > On Fri, Feb 12, 2016 at 08:45:43AM -0600, Josh Poimboeuf wrote:
> > > On Fri, Feb 12, 2016 at 11:36:24AM +0100, Jiri Slaby wrote:
> > >
> > > This seems like a real frame pointer bug caused by the following line in
> > > arch/x86/include/asm/preempt.h:
> > >
> > > # define __preempt_schedule() asm ("call ___preempt_schedule")
> >
> > The purpose there is that:
> >
> > preempt_enable();
> >
> > turns into:
> >
> > decl __percpu_prefix:__preempt_count
> > jnz 1f:
> > call ___preempt_schedule
> > 1:
> >
> > See arch/x86/include/asm/preempt.h:__preempt_count_dec_and_test()
>
> Sorry, I'm kind of confused. Do you mean that's what preempt_enable()
> would turn into *without* the above define?
>
> What I actually see in the listing is:
>
> decl __percpu_prefix:__preempt_count
> je 1f:
> ....
> 1:
> call ___preempt_schedule
>
> So it puts the "call ___preempt_schedule" in the slow path.
>
> I also don't see how that would be related to the use of the asm
> statement in the __preempt_schedule() macro. Doesn't the use of
> unlikely() in preempt_enable() put the call in the slow path?
>
> #define preempt_enable() \
> do { \
> barrier(); \
> if (unlikely(preempt_count_dec_and_test())) \
> preempt_schedule(); \
> } while (0)
>
> Also, why is the thunk needed? Any reason why preempt_enable() can't be
> called directly from C?

Sorry, s/preempt_enable/preempt_schedule/ on that last sentence.

--
Josh