[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/7] x86: Temporary disable SMAP to legally access user pages in kernel mode
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: Wednesday, April 23, 2014 10:10 PM > To: Wu, Feng > Cc: andrew.cooper3@xxxxxxxxxx; ian.campbell@xxxxxxxxxx; Dong, Eddie; > Nakajima, Jun; Tian, Kevin; xen-devel@xxxxxxxxxxxxx > Subject: RE: [PATCH v2 3/7] x86: Temporary disable SMAP to legally access user > pages in kernel mode > > >>> On 23.04.14 at 15:26, <feng.wu@xxxxxxxxx> wrote: > >> If done this way, this really should be moved to the out: label. But > >> I think the risk of missing a return path is quite high this way - I'd > >> much rather want the caller to wrap its call in a stac()/clac() pair. > > > > So, do you think code like the following looks better? > > > > stac(); > > construct_dom0(); > > clac(); > > Yes. > > >> > --- a/xen/arch/x86/x86_64/entry.S > >> > +++ b/xen/arch/x86/x86_64/entry.S > >> > @@ -380,6 +380,7 @@ __UNLIKELY_END(create_bounce_frame_bad_sp) > >> > movb TRAPBOUNCE_flags(%rdx),%cl > >> > subq $40,%rsi > >> > movq UREGS_ss+8(%rsp),%rax > >> > + ASM_STAC > >> > .Lft2: movq %rax,32(%rsi) # SS > >> > movq UREGS_rsp+8(%rsp),%rax > >> > .Lft3: movq %rax,24(%rsi) # RSP > >> > @@ -437,9 +438,11 @@ UNLIKELY_END(bounce_failsafe) > >> > testq %rax,%rax > >> > UNLIKELY_START(z, create_bounce_frame_bad_bounce_ip) > >> > lea > >> > > >> > UNLIKELY_DISPATCH_LABEL(create_bounce_frame_bad_bounce_ip)(%rip), %rd > >> i > >> > + ASM_CLAC > >> > jmp asm_domain_crash_synchronous /* Does not return > */ > >> > >> Interestingly here you spotted the need - you may want to consider > >> doing this at the asm_domain_crash_synchronous label instead. > > > > Yes, that should be a clean solution. But seems we don't need to call > > "ASM_CLAC" in > > all the places where asm_domain_crash_synchronous() is called. For example: > > > > In file xen/arch/x86/x86_64/entry.S: > > > > create_bounce_frame: > > ...... > > ...... > > > > UNLIKELY_START(g, create_bounce_frame_bad_sp) > > lea > UNLIKELY_DISPATCH_LABEL(create_bounce_frame_bad_sp)(%rip), > > %rdi > > jmp asm_domain_crash_synchronous /* Does not return */ > > __UNLIKELY_END(create_bounce_frame_bad_sp) > > Right, but the question you need to ask: Does it hurt anything? I > don't think so, and it covers all relevant cases in one go. It's a > code path that's being taken when something is wrong with the > guest anyway, so doing an clac in that case doesn't hurt a all. Yes, it doesn't hurt anything. > > Jan Thanks, Feng _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |