Re: [Qemu-devel] [PATCH v11 21/28] sev/i386: add debug encrypt and decrypt commands

[Brijesh Singh]

On 3/7/18 12:24 PM, Dr. David Alan Gilbert wrote:
> * Brijesh Singh (address@hidden) wrote:
>>
>> On 03/07/2018 11:27 AM, Dr. David Alan Gilbert wrote:
>>
>> [...]
>>
>>>> +{
>>>> +    SEVState *s = (SEVState *)handle;
>>>> +
>>>> +    /* If policy does not allow debug then no need to register ops */
>>>> +    if (s->policy & SEV_POLICY_NODBG) {
>>>> +        return;
>>>> +    }
>>> So what happens if someone tries to use a gdb or monitor command when
>>> policy didn't allow debug?  Does it end up with an obvious error
>>> somehow?
>>>
>> In those cases caller will get encrypted bytes, leading to unintelligible
>> data. It can sometime translate into  obvious errors e.g caller tries to
>> walk guest pagtable and it gets garbage and will have trouble dumping the
>> pgtables etc. Many times qemu calls ldphys_* functions to access the data it
>> may get tricky to report the errors.
> So would it make sense to have something like:
>
> sev_mem_cant_read(uint8_t *dst, const uint8_t *src, uint32_t len, MemTxAttrs 
> attrs)
> {
>     error_report("SEV Guest policy does not allow debug access");
>
>     return -EPERM;
> }

In very early patches we had something similar but I was not sure  if
that was right thing. Any debug accesses were  printing ton of messages
and also in some case caller actually wants to dump the memory content
(e.g x /10gx 0x000).. what we should return in those cases ? In my
approach was if debug was not enabled then simply don't decrypt the
memory and provide the raw data.

There was some discussion to have very high level security policy which
may have attribute like debug=on|off, if debug is disabled then QEMU
monitor can display messages like debug not allowed to better inform user.


> void
> sev_set_debug_ops(void *handle, MemoryRegion *mr)
> {
>     SEVState *s = (SEVState *)handle;
>
>     /* If policy does not allow debug then no need to register ops */
>     if (s->policy & SEV_POLICY_NODBG) {
>         sev_ops.read = sev_mem_cant_read;
>         sev_ops.write = sev_mem_cant_write;
>     } else {
>         sev_ops.read = sev_mem_read;
>         sev_ops.write = sev_mem_write;
>     }
>
> Dave
>
>> -Brijesh
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK