Re: [Part2 PATCH v6 32/38] KVM: SVM: Add support for SEV DEBUG_DECRYPT command

[Brijesh Singh]

On 10/30/17 10:12 AM, Borislav Petkov wrote:
...
> Lemme see:
>
> sev_dbg_crypt() does
>
>                 ret = __sev_dbg_decrypt(kvm,
>                                        __sme_page_pa(src_p[0]) + s_off,
>                                        dst_vaddr, 0,
>                                        __sme_page_pa(dst_p[0]) + d_off,
>                                        len, &argp->error);
>
> and that 4th argument is 0. IINM, that's dst_kaddr and you're doing
>
> 	memcpy((void *)dst_kaddr, page_address(tpage) + offset, size);
> 		^^^^^^^^^^^^^^^^

The 3rd argument 'dst_uaddr' should always contain a valid value and 4th
argument should not be used. The else statement should not be used
during DBG_DECRYPT command.

ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ /*
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ * If destination buffer is a userspace buffer then use
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ * copy_to_user otherwise memcpy.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ */
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (dst_uaddr) {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (copy_to_user((void __user
*)(uintptr_t)dst_uaddr,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ page_address(tpage) + offset,
size))
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ret = -EFAULT;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ } else {
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ memcpy((void *)dst_kaddr, page_address(tpage) +
offset, size);
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }


Here is sequence

sev_dbg_crypt() does:

   dst_vaddr = params.dst_uaddr;
ÂÂÂÂÂ .......
ÂÂÂÂÂ for(...) {
ÂÂÂÂ ÂÂÂ ÂÂ dst_p = sev_pin_memory(..., dst_vaddr ...)
ÂÂÂ ÂÂÂ ÂÂÂ if (!dst_p) {
ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ return -EFAULT;
ÂÂÂ ÂÂÂ ÂÂÂÂ }

ÂÂÂ ÂÂÂ ÂÂÂ

	ret = __sev_dbg_decrypt(kvm,
                                __sme_page_pa(src_p[0]) + s_off,
                                dst_vaddr, 0,
                                __sme_page_pa(dst_p[0]) + d_off,
                                len, &argp->error);

....


The 3rd argument will be zero when we are handling the DBG_ENCRYPTÂ
with length not aligned to 16-byte boundary. In that case we allocate a
intermediate buffer (dst_kaddr). I will try with gcc7 and look into
restructure code to fix the compiler warning.