[svsm-devel] x86 decoder further steps

[Carlos López]

Hi,

On 30/4/24 11:56, Thomas Leroy wrote:
> Hi everyone,
> 
> As you may know, the current #VC handler uses a naive x86 instruction
> decoder
> that is used to find the appropriate NAE event handler to call.
> 
> As I said, the current decoder is very naive and does only byte matching to
> identify cpuid, in/out{bwl}, rdmsr/wrmsr, and rdtsc{p} instructions [0].
> Since we currently don't support all NAE events in the #VC handler, we have
> to find a clean way to decode the remaining NAEs.

Yes, it will require at least a small rework to handle instruction
prefixes (namely REP).

> If the decoder's only use is for the #VC handler, I guess we can stick
> to what
> we currently do. It shouldn't be too hard to match the remaining NAE
> instructions
> in the decoder. However, if we need a complete instruction decoder
> somewhere,
> we'll have to decide if we write our own or if we use an existing one.
> I'm not aware of the full SVSM development plan, so I don't really know the
> possible other uses of the decoder.
> 
> As you may also noticed, the instruction decoder is an attack surface
> since it
> deals with untrusted raw bytes. For this reason, we should make it as small
> (and secure of course) as possible. So if we finally need a complete
> decoder,
> we'll have to choose between write our own, which will be time consuming and
> could introduce security bugs (especially when #VC exceptions will be raised
> from a less privileged context), or rely on a third-party one (that we
> shouldn't consider prefect security-wise).
> 
> What's your opinion between these solutions?
> 
> I recently discovered the iced-x86 [1] crate looks easy to incorporate
> to the
> SVSM project, and supports no_std, so I think it would be a candidate.
> Finally, if we go for a third-party one, a security audit of the chosen
> decoder would be needed, at least a fuzzing campaign (I could take care
> of it).

I haven't attempted to the decode the remaining instructions, but other
than the prefix issue I mentioned above I don't think we need much more.
On the other hand, iced-x86 has a lot of features that can be disabled,
since we only want instruction decoding, so we wouldn't be importing
that much code. The project also already has fuzzing, so I assume it's
well tested overall.

> Cheers,
> Thomas
> 
> [0]
> https://github.com/coconut-svsm/svsm/blob/104ca723cdd6a272f40c31548dd6c05010494c84/kernel/src/cpu/insn.rs#L110
> 
> [1] https://docs.rs/iced-x86/latest/iced_x86/
> 

-- 
Carlos López
Security Engineer
SUSE Software Solutions