lkml.org 
[lkml]   [2014]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC 3/5] x86: Add a comment clarifying LDT context switching
On Thu, Oct 16, 2014 at 8:49 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Oct 14, 2014 at 03:57:37PM -0700, Andy Lutomirski wrote:
>> The code is correct, but only for a rather subtle reason. This
>> confused me for quite a while when I read switch_mm, so clarify the
>> code to avoid confusing other people, too.
>>
>> TBH, I wouldn't be surprised if this code was only correct by
>> accident.
>>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> ---
>> arch/x86/include/asm/mmu_context.h | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
>> index 166af2a8e865..04478103df37 100644
>> --- a/arch/x86/include/asm/mmu_context.h
>> +++ b/arch/x86/include/asm/mmu_context.h
>> @@ -53,7 +53,16 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>> /* Stop flush ipis for the previous mm */
>> cpumask_clear_cpu(cpu, mm_cpumask(prev));
>>
>> - /* Load the LDT, if the LDT is different: */
>> + /*
>> + * Load the LDT, if the LDT is different.
>> + *
>> + * It's possible leave_mm(prev) has been called. If so,
>> + * then prev->context.ldt could be out of sync with the
>> + * LDT descriptor or the LDT register. This can only happen
>
> I'm staring at the code and trying to figure out where on the leave_mm()
> path this could happen. Got any code pointers?

I think it's the same as in the other case in switch_mm. leave_mm
does cpumask_clear_cpu(cpu, mm_cpumask(active_mm)), and, once that has
happened, modify_ldt won't send an IPI to this CPU. So, if leave_mm
runs, and then another CPU calls modify_ldt on the mm that is in lazy
mode here, it won't update our LDT register, so the LDT register and
prev->context.ldt might not match.

I think that, if that's the case, then prev->context.ldt can't be NULL
and can't have the same non-NULL value as next->context.ldt. I hope.

I'll try to make the comment clearer in v2.

--Andy

>
> :-)
>
>> + * if prev->context.ldt is non-null, since we never free
>> + * an LDT. But LDTs can't be shared across mms, so
>> + * prev->context.ldt won't be equal to next->context.ldt.
>> + */
>> if (unlikely(prev->context.ldt != next->context.ldt))
>> load_LDT_nolock(&next->context);
>> }
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
> --
> Regards/Gruss,
> Boris.
> --



--
Andy Lutomirski
AMA Capital Management, LLC


\
 
 \ /
  Last update: 2014-10-16 19:01    [W:0.094 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site