"do_iret" (slow iret via hyercall) can introduce a race condition as
"current" can change during the execution of the function. all
hypercalls
run with "sti" on, so an interrupt on a processor causing the control
to
enter in "__enter_scheduler" after reading current can change the
current
process on that processor.
code excerpt
"
struct iret_context iret_saved;
struct vcpu *v = current;
if ( unlikely(copy_from_user(&iret_saved, (void *)regs->rsp,
sizeof(iret_saved))) )
{
"
Any thoughts on this?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Tue, 2007-11-27 at 13:59 -0500, Ashish Bijlani wrote:> "do_iret" (slow iret via hyercall) can introduce a race condition as > "current" can change during the execution of the function. all > hypercalls run with "sti" on, so an interrupt on a processor causing > the control to enter in "__enter_scheduler" after reading current can > change the current process on that processor.follow the code e.g. surounding ret_from_intr. __enter_scheduler is a softirq handler. it will only be run upon return to guest context, not when returning to an interrupted hypervisor. xen is not preemptible. regards, daniel -- Daniel Stodden LRR - Lehrstuhl für Rechnertechnik und Rechnerorganisation Institut für Informatik der TU München D-85748 Garching http://www.lrr.in.tum.de/~stodden mailto:stodden@cs.tum.edu PGP Fingerprint: F5A4 1575 4C56 E26A 0B33 3D80 457E 82AE B0D8 735B _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
since "sti" is on, an interrupt can occur which can cause the control to enter "__runq_tickle", which then invokes "cpumask_raise_softirq(mask, SCHEDULE_SOFTIRQ)" to send IPI to a processor causing the control to enter "__enter_scheduler" which changes the current. On Nov 27, 2007 2:16 PM, Daniel Stodden <stodden@cs.tum.edu> wrote:> > On Tue, 2007-11-27 at 13:59 -0500, Ashish Bijlani wrote: > > "do_iret" (slow iret via hyercall) can introduce a race condition as > > "current" can change during the execution of the function. all > > hypercalls run with "sti" on, so an interrupt on a processor causing > > the control to enter in "__enter_scheduler" after reading current can > > change the current process on that processor. > > follow the code e.g. surounding ret_from_intr. __enter_scheduler is a > softirq handler. it will only be run upon return to guest context, not > when returning to an interrupted hypervisor. xen is not preemptible. > > regards, > daniel > > -- > Daniel Stodden > LRR - Lehrstuhl für Rechnertechnik und Rechnerorganisation > Institut für Informatik der TU München D-85748 Garching > http://www.lrr.in.tum.de/~stodden <http://www.lrr.in.tum.de/%7Estodden> > mailto:stodden@cs.tum.edu > PGP Fingerprint: F5A4 1575 4C56 E26A 0B33 3D80 457E 82AE B0D8 735B > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
raise_softirq is not the same as actually doing it. Raise only sets a
bit indicating pending softirq. Before returning to the guest, the bin
triggers actual execution of the softirq
Andres
since "sti" is on, an interrupt can occur which can cause the
control to enter "__runq_tickle", which then invokes
"cpumask_raise_softirq(mask, SCHEDULE_SOFTIRQ)" to send IPI to a
processor causing the control to enter "__enter_scheduler" which
changes the current.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
"cpumask_raise_softirq(mask, SCHEDULE_SOFTIRQ)" sends an IPI to the target processor, this can cause current to change. On Nov 27, 2007 4:50 PM, Andres Lagar-Cavilla <andreslc@cs.toronto.edu> wrote:> raise_softirq is not the same as actually doing it. Raise only sets a > bit indicating pending softirq. Before returning to the guest, the bin > triggers actual execution of the softirq > > Andres > > since "sti" is on, an interrupt can occur which can cause the > control to enter "__runq_tickle", which then invokes > "cpumask_raise_softirq(mask, SCHEDULE_SOFTIRQ)" to send IPI to a > processor causing the control to enter "__enter_scheduler" which > changes the current. > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, 2007-11-27 at 17:41 -0500, Ashish Bijlani wrote:> "cpumask_raise_softirq(mask, SCHEDULE_SOFTIRQ)" sends an IPI to the > target processor, this can cause current to change.No it can''t. The IPI causes smp_event_check_interrupt() to be called which just ACKs the IPI and returns via ret_from_intr (in entry.S). It is only if a guest was interrupted that we go down the test_all_events path which processes softirqs. If it was Xen which was interrupted then we go to restore_all_xen which just returns to Xen. In this case Xen will eventually return to the guest and take the test_all_events path and process the softirq. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
yeah but the do_iret function is done on behalf of a guest, therefore
do_iret function forces user cs and user ss
code excerpt
"
regs->rip = iret_saved.rip;
regs->cs = iret_saved.cs | 3; /* force guest privilege */
regs->rflags = (iret_saved.rflags & ~(EF_IOPL|EF_VM)) | EF_IE;
regs->rsp = iret_saved.rsp;
regs->ss = iret_saved.ss | 3; /* force guest privilege */
"
this can cause ret_from_intr go to test_all_events and finally go to
__enter_scheduler
-a
On Nov 27, 2007 6:17 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>
> On Tue, 2007-11-27 at 17:41 -0500, Ashish Bijlani wrote:
> > "cpumask_raise_softirq(mask, SCHEDULE_SOFTIRQ)" sends an IPI
to the
> > target processor, this can cause current to change.
>
> No it can''t.
>
> The IPI causes smp_event_check_interrupt() to be called which just ACKs
> the IPI and returns via ret_from_intr (in entry.S). It is only if a
> guest was interrupted that we go down the test_all_events path which
> processes softirqs. If it was Xen which was interrupted then we go to
> restore_all_xen which just returns to Xen. In this case Xen will
> eventually return to the guest and take the test_all_events path and
> process the softirq.
>
> Ian.
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Tue, 2007-11-27 at 18:30 -0500, Ashish Bijlani wrote:> yeah but the do_iret function is done on behalf of a guest, therefore > do_iret function forces user cs and user ss > > code excerpt > " > regs->rip = iret_saved.rip; > regs->cs = iret_saved.cs | 3; /* force guest privilege */ > regs->rflags = (iret_saved.rflags & ~(EF_IOPL|EF_VM)) | EF_IE; > regs->rsp = iret_saved.rsp; > regs->ss = iret_saved.ss | 3; /* force guest privilege */ > " > this can cause ret_from_intr go to test_all_events and finally go to > __enter_schedulerthat''s the guest context saved in _memory_ (xen stack) which gets modified -- user or kernel. that''s where what it ultimately returns _to_ upon return _from_ do_iret. with an interrupted do_iret, the switch in ret_from_intr would be yet another stack frame above, and that would be xen being interrupted. regards, daniel -- Daniel Stodden LRR - Lehrstuhl für Rechnertechnik und Rechnerorganisation Institut für Informatik der TU München D-85748 Garching http://www.lrr.in.tum.de/~stodden mailto:stodden@cs.tum.edu PGP Fingerprint: F5A4 1575 4C56 E26A 0B33 3D80 457E 82AE B0D8 735B _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
current is extracted from per processor stack area. so are the registers. right? do_iret gets current and regs from the processor''s stack area. so does ret_from_intr. so they both point to a fixed per processor stack area. there are not _different_ stack frames. On Nov 27, 2007 7:51 PM, Daniel Stodden < stodden@cs.tum.edu> wrote:> > On Tue, 2007-11-27 at 18:30 -0500, Ashish Bijlani wrote: > > yeah but the do_iret function is done on behalf of a guest, therefore > > do_iret function forces user cs and user ss > > > > code excerpt > > " > > regs->rip = iret_saved.rip; > > regs->cs = iret_saved.cs | 3; /* force guest privilege */ > > regs->rflags = (iret_saved.rflags & ~(EF_IOPL|EF_VM)) | EF_IE; > > regs->rsp = iret_saved.rsp; > > regs->ss = iret_saved.ss | 3; /* force guest privilege */ > > " > > this can cause ret_from_intr go to test_all_events and finally go to > > __enter_scheduler > > that''s the guest context saved in _memory_ (xen stack) which gets > modified -- user or kernel. that''s where what it ultimately returns _to_ > upon return _from_ do_iret. with an interrupted do_iret, the switch in > ret_from_intr would be yet another stack frame above, and that would be > xen being interrupted. > > regards, > daniel > > -- > Daniel Stodden > LRR - Lehrstuhl für Rechnertechnik und Rechnerorganisation > Institut für Informatik der TU München D-85748 Garching > http://www.lrr.in.tum.de/~stodden <http://www.lrr.in.tum.de/%7Estodden> > mailto:stodden@cs.tum.edu > PGP Fingerprint: F5A4 1575 4C56 E26A 0B33 3D80 457E 82AE B0D8 735B > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, 2007-11-27 at 20:15 -0500, Ashish Bijlani wrote:> current is extracted from per processor stack area. so are the > registers. right?yes.> do_iret gets current and regs from the processor''s stack area. so does > ret_from_intr.yes. but the latter from a different stack *frame* in the case you''re concerned about. same stack, though.> so they both point to a fixed per processor stack area. there are not > _different_ stack frames.i see what you mean. let''s look at the stack when an IPI occurs to trigger a context switch. --snip-- guest calls __HYPERVISOR_iret (saves guest cs) do_iret tinkers with guest_cpu_user_regs IPI caught (saves xen cs) ret_from_intr tests xen (!!) cs, not guest cs do_iret continues and finishes. same current. test_all_events calls do_softirq. schedules. new current. return to new current. --snap-- UREGS_cs in entry.S refers to the interrupted context, not the saved guest context. this may be the same: the IPI would have mattered if it interrupted the guest. it did not. so the softirq will only be run before returning to the guest, not in between. regards, daniel -- Daniel Stodden LRR - Lehrstuhl für Rechnertechnik und Rechnerorganisation Institut für Informatik der TU München D-85748 Garching http://www.lrr.in.tum.de/~stodden mailto:stodden@cs.tum.edu PGP Fingerprint: F5A4 1575 4C56 E26A 0B33 3D80 457E 82AE B0D8 735B _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel