Re: [PATCH] vm_event: Implement ARM SMC events

[Corneliu ZUZU <czuzu@bitdefender.com>]

[-- Attachment #1.1: Type: text/plain, Size: 10044 bytes --]

On 4/12/2016 8:24 PM, Tamas K Lengyel wrote:
>
>
> On Tue, Apr 12, 2016 at 11:05 AM, Corneliu ZUZU <czuzu@bitdefender.com 
> <mailto:czuzu@bitdefender.com>> wrote:
>
>     On 4/12/2016 7:24 PM, Julien Grall wrote:
>
>         Hello,
>
>         On 12/04/2016 16:01, Tamas K Lengyel wrote:
>
>
>             On Apr 12, 2016 01:51, "Corneliu ZUZU"
>             <czuzu@bitdefender.com <mailto:czuzu@bitdefender.com>
>             <mailto:czuzu@bitdefender.com
>             <mailto:czuzu@bitdefender.com>>> wrote:
>              >
>              > On 4/11/2016 10:47 PM, Tamas K Lengyel wrote:
>              >>
>              >> From: Tamas K Lengyel <tklengyel@sec.in.tum.de
>             <mailto:tklengyel@sec.in.tum.de>
>             <mailto:tklengyel@sec.in.tum.de
>             <mailto:tklengyel@sec.in.tum.de>>>
>              >>
>              >> The ARM SMC instructions are already configured to
>             trap to Xen by
>             default. In
>              >> this patch we allow a user-space process in a
>             privileged domain to
>             receive
>              >> notification of when such event happens through the
>             vm_event subsystem.
>              >>
>              >> This patch will likely needs to be broken up into
>             several smaller
>             patches.
>              >> Right now what this patch adds (and could be broken
>             into smaller patches
>              >> accordingly):
>              >>      - Implement monitor_op domctl handler for
>             SOFTWARE_BREAKPOINTs
>             on ARM
>              >>      - Implement vm_event register fill/set routines
>             for ARM. This
>             required
>              >>          removing the function from common as the
>             function prototype now
>              >>          differs on the two archs.
>              >>      - Sending notification as SOFTWARE_BREAKPOINT
>             vm_event from the
>             SMC trap
>              >>          handlers.
>              >>      - Extend the xen-access test tool to receive SMC
>             notification
>             and step
>              >>          the PC manually in the reply.
>              >>
>              >> I'm sending it as an RFC to gather feedback on what
>             has been
>             overlooked in this
>              >> revision. This patch has been tested on a Cubietruck
>             board and works
>             fine,
>              >> but would probably not work on 64-bit boards.
>              >
>              >
>              > Hi Tamas,
>              >
>              > If I may, I'm still unable to work at the moment, being
>             ill, but I'm
>             checking the xen-devel lists from time to time.
>              > Your patch caught my attention, reminding me of the
>             conversation we
>             had some time ago on this matter.
>              > The only real reason I don't see SMC
>             (secure-monitor-call) as being
>             an ideal candidate for this is that, according to the ARM
>             manuals, SMC
>             should directly cause undefined exception if executed from
>             user-mode
>             (EL0), instead of a hypervisor trap - isn't that the case
>             on the machine
>             you tested this on or is this really only for the EL1 of
>             domains?
>
>
>         This paragraph is part of Corneliu's answer but it gives the
>         impression you wrote it. Can you configure your e-mail client
>         to quote properly?
>
>
>             That's correct, it can only be issued by the kernel. So as
>             long as you
>             want to monitor the kernel it can be used just fine. I can
>             also envision
>             trampoline-like traps (syscalls injected into EL0 to
>             trigger SMC) but
>             that's beyond the scope I intend this for now.
>
>
>     Then indeed SMC is the -easiest- way to go, @ least until
>     user-mode breakpoints are implemented.
>
>
>              >
>              > Also:
>              > - SMC, by definition, is a call to the secure side, it
>             doesn't relate
>             to debugging directly (it's a syscall to the 'secure'
>             side). There is a
>             viable INT3 equivalent on ARM, that being the BKPT/BRK
>             instruction,
>             using that instead would require a bit more effort (but would,
>             conceptually, be more correct) and might be less
>             performant, I suppose
>             that's why you didn't go for that?
>
>
>         BKPT/BRK could be used by the guest for debugging. You would
>         need to differentiate a breakpoint inserted by Xen or by a
>         debugger in the guest.
>
>
>     Isn't that also the case for X86's INT3? I guess differentiation
>     is done based on the bkpt address/privilege level? On ARM that
>     could also (partially) be done by looking @ the immediate value of
>     the BKPT/BRK instruction. Another thing I realized might be
>     troublesome with NOT using BKPT/BRK would be that on ARMv7 BKPT is
>     always unconditional, even in IT blocks. IDK if that applies to
>     SMC, but if it doesn't you'd have to check for that as well to
>     make sure the breakpoint is actually executed.
>
>
>
>             I would have to double check but AFAIK those instructions
>             can't be
>             configured to trap to the hypervisor directly. So while
>             SMC was not
>             intended to be a breakpoint, conceptually it's the closest
>             thing we have
>             an on ARM to the INT3 instruction when configured to trap
>             to the VMM.
>
>
>
>     Please see AArch32 HDCR.TDE and AArch64 MDCR_EL2.TDE bits. Since
>     activating this bit would imply additional (in this context
>     -unwanted-) traps, the performance hit of having this bit set
>     might be significant.
>
>
> Right, actually I believe KVM already implemented this, I was just 
> under the impression it was only for aarch64. As for performance 
> overhead I'm not that worried about it, rather I need to make sure the 
> presence of the monitoring can remain stealthy. I know on KVM for 
> example this type of trapping renders the guest to be unable to use 
> singlestepping, which would easily reveal the presence of the external 
> monitor (see 
> https://lists.cs.columbia.edu/pipermail/kvmarm/2015-May/014589.html). 
> So this will need to be looked at carefully.

That seems to apply to single-stepping only, which would be a different 
matter. As for stealthiness or not limiting the guest, IMO that 
shouldn't be a problem with BKPT/BRK, since I believe you can inject the 
breakpoint exception into the guest as if no hypervisor trap occured in 
between (of course, once you decide whether that breakpoint is Xen's or 
guest-internal). But what about X86? How is stealthiness achieved there? 
Is INT3 entirely not available for the guest anymore when 
guest-debugging is enabled or are ALL INT3's reported by Xen as software 
breakpoint vm-events?

>
>         Whilst any access to SMC currently results to inject an
>         undefined exception, it may not be the case in the future.
>         There have been discussion to allow guest issuing SMC call
>         (see [1]).
>
>
> I don't see a problem with that, as long as it's configurable whether 
> SMC calls are trapped or pass-through.

Pass-through meaning "not trapped at all"? If yes, the problem would be 
that you won't be able to set breakpoints when SMC is configured to 
"completely" pass-through. But there's also the option of emulating the 
SMC, instead of not trapping it at all, when pass-through is needed, 
although IDK how complex that emulation would be.

>
>         I think the safest instruction would be HVC #imm. Xen is only
>         using a small number of immediate. You could allocate a
>         specific value for software debugging.
>

Another issue came to my mind: "HVC #imm", if handled through the 
hvm-ops code, currently requires setting other registers to predefined 
values before the HVC is actually issued. That would imply additional 
effort to save/restore those registers if an external privileged domain 
would want to set guest breakpoints. Given that, if we were to use HVC 
for sw-bkpts, IMO it would be nice if the hvm-ops code architecture 
would be slightly changed such that -lone- "HVM #imm" calls would be 
achievable for some use cases, such as this.

>
>
>     IMHO that would also be better conceptually, although it would
>     still suffer from the limitation of not being available from
>     user-space (and potentially from the above IT block issue).
>
>
> Sure, HVC would also be a possibility but I do see use-cases for 
> trapping SMC calls and forwarding them to a guest instead of the TZ. 
> For example, if malware tries to exploit TZ vulnerabilities, it would 
> be a lot easier to contain and monitor such exploits if the TZ is 
> virtualized rather then just crashing the guest or forwarding the 
> calls to a real TZ. So trapping SMC would allow us to use the real TZ 
> for other purposes and maintain a barrier between malicious guests and 
> the TZ.

Then you'd have to differentiate between a genuine guest SMC and a 
software-breakpoint SMC. IDK how much of a problem that would be.

>
> So what I will do instead of issuing a software_breakpoint vm_event 
> for SMCs, I'll introduce a new type, say 
> VM_EVENT_REASON_PRIVILEGED_CALL, that can be used to forward both 
> hypercalls and SMCs to a monitoring guest. This would also allow us to 
> use the software_breakpoint type for the actual software breakpoint 
> events in the future.
>
> Tamas

Isn't the HVC-part already achieved by guest-request vm-events? Maybe 
tying this vm-event specifically to SMC (in which case the name could be 
something like VM_EVENT_REASON_SECURE_CALL) and thus making it 
ARM-specific would avoid that redundancy?

Cheers,
Corneliu.

[-- Attachment #1.2: Type: text/html, Size: 18568 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel