Re: [PATCH v4 5/5] x86, traps: Add ist_begin_non_atomic and ist_end_non_atomic

[Andy Lutomirski]

On Mon, Nov 24, 2014 at 7:54 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Fri, Nov 21, 2014 at 01:26:11PM -0800, Andy Lutomirski wrote:
>> In some IST handlers, if the interrupt came from user mode,
>> we can safely enable preemption.  Add helpers to do it safely.
>>
>> This is intended to be used my the memory failure code in
>> do_machine_check.
>>
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
>> ---
>>  arch/x86/include/asm/traps.h |  2 ++
>>  arch/x86/kernel/traps.c      | 38 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 40 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
>> index eb16a61bfd06..04ba537fc721 100644
>> --- a/arch/x86/include/asm/traps.h
>> +++ b/arch/x86/include/asm/traps.h
>> @@ -112,6 +112,8 @@ asmlinkage void mce_threshold_interrupt(void);
>>
>>  extern enum ctx_state ist_enter(struct pt_regs *regs);
>>  extern void ist_exit(struct pt_regs *regs, enum ctx_state prev_state);
>> +extern void ist_begin_non_atomic(struct pt_regs *regs);
>> +extern void ist_end_non_atomic(void);
>>
>>  /* Interrupts/Exceptions */
>>  enum {
>> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
>> index 6a02760df7b4..2b5f2e038e3f 100644
>> --- a/arch/x86/kernel/traps.c
>> +++ b/arch/x86/kernel/traps.c
>> @@ -140,6 +140,44 @@ void ist_exit(struct pt_regs *regs, enum ctx_state prev_state)
>>               rcu_nmi_exit();
>>  }
>>
>> +/**
>> + * ist_begin_non_atomic() - begin a non-atomic section in an IST exception
>> + * @regs:    regs passed to the IST exception handler
>> + *
>> + * IST exception handlers normally cannot schedule.  As a special
>> + * exception, if the exception interrupted userspace code (i.e.
>> + * user_mode_vm(regs) would return true) and the exception was not
>> + * a double fault, it can be safe to schedule.  ist_begin_non_atomic()
>> + * begins a non-atomic section within an ist_enter()/ist_exit() region.
>> + * Callers are responsible for enabling interrupts themselves inside
>> + * the non-atomic section, and callers must call is_end_non_atomic()
>> + * before ist_exit().
>> + */
>
> Ok, I guess this is is fine, albeit a bit circumstantial:
>
> I need to do
>
>         ist_enter()
>         ist_begin_non_atomic()
>
> In here, I still need to do
>
>         if (user_mode_vm()) {
>                 # do non-atomic stuff
>         }
>
> AFAICT, right?
>
>         ist_end_non_atomic()
>         ist_exit()
>
> and this whole fun for the context tracking exception_enter/_exit()
> calls.

Not quite.  This fun stuff is to satisfy Linus' request to make the
atomicness of the IST entries unconditional instead of depending on
the weird user_space_vm thing and to make the escape to non-atomicness
be clearly a special case.

So your code above should OOPS instead of working.  The code should be:

ist_enter()

if (user_mode_vm()) {
  ist_begin_non_atomic();
  local_irq_enable();
  # do non-atomic stuff
  local_irq_disable();
  ist_end_non_atomic();
}

ist_exit().

Although that user_mode_vm() check might be subsumed by the mce_severity stuff.

NB: This series is going to look a bit different once I rebase it.
This part is the same, but Linus merged some patches that change some
of the same code over the weekend.

--Andy

>
> Acked-by: Borislav Petkov <bp@xxxxxxx>
>
> --
> Regards/Gruss,
>     Boris.
>
> Sent from a fat crate under my desk. Formatting is fine.
> --



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/