[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrV6QUCVR+6_CCQpJQj55F7vao8qGBXd1wJOG+M+W4FGeA@mail.gmail.com>
Date: Mon, 23 Mar 2015 12:10:04 -0700
From: Andy Lutomirski <luto@...capital.net>
To: Denys Vlasenko <dvlasenk@...hat.com>
Cc: Takashi Iwai <tiwai@...e.de>,
Denys Vlasenko <vda.linux@...glemail.com>,
Jiri Kosina <jkosina@...e.cz>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Stefan Seyfried <stefan.seyfried@...glemail.com>,
X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
Tejun Heo <tj@...nel.org>
Subject: Re: PANIC: double fault, error_code: 0x0 in 4.0.0-rc3-2, kvm related?
On Mon, Mar 23, 2015 at 12:07 PM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
> On 03/23/2015 07:38 PM, Andy Lutomirski wrote:
>>> cmpq $__NR_syscall_max,%rax
>>> ja ret_from_sys_call
>>> movq %r10,%rcx
>>> call *sys_call_table(,%rax,8) # XXX: rip relative
>>> movq %rax,RAX-ARGOFFSET(%rsp)
>>> ret_from_sys_call:
>>> testl $_TIF_ALLWORK_MASK,TI_flags+THREAD_INFO(%rsp,RIP-ARGOFFSET)
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> jnz int_ret_from_sys_call_fixup /* Go the the slow path */
>>> LOCKDEP_SYS_EXIT
>>> DISABLE_INTERRUPTS(CLBR_NONE)
>>> TRACE_IRQS_OFF
>>> ...
>>> ...
>>> int_ret_from_sys_call_fixup:
>>> FIXUP_TOP_OF_STACK %r11, -ARGOFFSET
>>> jmp int_ret_from_sys_call
>>> ...
>>> ...
>>> GLOBAL(int_ret_from_sys_call)
>>> DISABLE_INTERRUPTS(CLBR_NONE)
>>> TRACE_IRQS_OFF
>>>
>>> You reverted that by moving this insn to be after first DISABLE_INTERRUPTS(CLBR_NONE).
>>>
>>> I also don't see how moving that check (even if it is wrong in a more
>>> benign way) can have such a drastic effect.
>>
>> I bet I see it. I have the advantage of having stared at KVM code and
>> cursed at it more recently than you, I suspect. KVM does awful, awful
>> things to CPU state, and, as an optimization, it allows kernel code to
>> run with CPU state that would be totally invalid in user mode. This
>> happens through a bunch of hooks, including this bit in __switch_to:
>>
>> /*
>> * Now maybe reload the debug registers and handle I/O bitmaps
>> */
>> if (unlikely(task_thread_info(next_p)->flags & _TIF_WORK_CTXSW_NEXT ||
>> task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
>> __switch_to_xtra(prev_p, next_p, tss);
>>
>> IOW, we *change* tif during context switches.
>>
>>
>> The race looks like this:
>>
>> testl $_TIF_ALLWORK_MASK,TI_flags+THREAD_INFO(%rsp,RIP)
>> jnz int_ret_from_sys_call_fixup /* Go the the slow path */
>>
>> --- preempted here, switch to KVM guest ---
>>
>> KVM guest enters and screws up, say, MSR_SYSCALL_MASK. This wouldn't
>> happen to be a *32-bit* KVM guest, perhaps?
>>
>> Now KVM schedules, calling __switch_to. __switch_to sets
>> _TIF_USER_RETURN_NOTIFY.
>
> Clear up to now...
>
>> We IRET back to the syscall exit code,
>
> So we end up being just after the "testl", right?
> We go into "int_ret_from_sys_call_fixup".
Nope, other way around. We saw no work bits set in testl, but one or
more of those bits was set when we're preempted and return. Now we
*don't* go to int_ret_from_sys_call_fixup. I don't think that the
resulting sysret itself is harmful, but I think we're now running user
code with some MSRs programmed wrong. The next syscall could do bad
things, such as failing to clear IF.
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists