From mboxrd@z Thu Jan  1 00:00:00 1970
From: Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [Part2 PATCH v6 32/38] KVM: SVM: Add support for SEV
 DEBUG_DECRYPT command
Date: Mon, 30 Oct 2017 11:33:54 -0500
Message-ID: <2b5499db-7752-7919-5e12-25475fe6372f@amd.com>
References: <20171020023413.122280-1-brijesh.singh@amd.com>
 <20171020023413.122280-33-brijesh.singh@amd.com>
 <20171027202518.GS12039@nazgul.tnic>
 <5e041a3f-823f-08ce-a653-a02b71e2cb6b@amd.com>
 <20171030151223.vdxb6oc3cttw74zm@pd.tnic>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: brijesh.singh@amd.com, kvm@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        Joerg Roedel <joro@8bytes.org>,
        Tom Lendacky <thomas.lendacky@amd.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org
To: Borislav Petkov <bp@alien8.de>
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20171030151223.vdxb6oc3cttw74zm@pd.tnic>
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org



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.