linux-kernel - Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 Mar 2017 12:03:52 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Brijesh Singh <brijesh.singh@....com>, simon.guinot@...uanux.org,
        linux-efi@...r.kernel.org, kvm@...r.kernel.org, rkrcmar@...hat.com,
        matt@...eblueprint.co.uk, linux-pci@...r.kernel.org,
        linus.walleij@...aro.org, gary.hook@....com, linux-mm@...ck.org,
        paul.gortmaker@...driver.com, hpa@...or.com, cl@...ux.com,
        dan.j.williams@...el.com, aarcange@...hat.com,
        sfr@...b.auug.org.au, andriy.shevchenko@...ux.intel.com,
        herbert@...dor.apana.org.au, bhe@...hat.com, xemul@...allels.com,
        joro@...tes.org, x86@...nel.org, peterz@...radead.org,
        piotr.luc@...el.com, mingo@...hat.com, msalter@...hat.com,
        ross.zwisler@...ux.intel.com, bp@...e.de, dyoung@...hat.com,
        thomas.lendacky@....com, jroedel@...e.de, keescook@...omium.org,
        arnd@...db.de, toshi.kani@....com, mathieu.desnoyers@...icios.com,
        luto@...nel.org, devel@...uxdriverproject.org, bhelgaas@...gle.com,
        tglx@...utronix.de, mchehab@...nel.org, iamjoonsoo.kim@....com,
        labbott@...oraproject.org, tony.luck@...el.com,
        alexandre.bounine@....com, kuleshovmail@...il.com,
        linux-kernel@...r.kernel.org, mcgrof@...nel.org, mst@...hat.com,
        linux-crypto@...r.kernel.org, tj@...nel.org,
        akpm@...ux-foundation.org, davem@...emloft.net
Subject: Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT
 command



On 02/03/2017 16:18, Brijesh Singh wrote:
> +	data = (void *) get_zeroed_page(GFP_KERNEL);

The page does not need to be zeroed, does it?

> +
> +	if ((len & 15) || (dst_addr & 15)) {
> +		/* if destination address and length are not 16-byte
> +		 * aligned then:
> +		 * a) decrypt destination page into temporary buffer
> +		 * b) copy source data into temporary buffer at correct offset
> +		 * c) encrypt temporary buffer
> +		 */
> +		ret = __sev_dbg_decrypt_page(kvm, dst_addr, data, &argp->error);

Ah, I see now you're using this function here for read-modify-write.
data is already pinned here, so even if you keep the function it makes
sense to push pinning out of __sev_dbg_decrypt_page and into
sev_dbg_decrypt.

> +		if (ret)
> +			goto err_3;
> +		d_off = dst_addr & (PAGE_SIZE - 1);
> +
> +		if (copy_from_user(data + d_off,
> +					(uint8_t *)debug.src_addr, len)) {
> +			ret = -EFAULT;
> +			goto err_3;
> +		}
> +
> +		encrypt->length = PAGE_SIZE;

Why decrypt/re-encrypt all the page instead of just the 16 byte area
around the [dst_addr, dst_addr+len) range?

> +		encrypt->src_addr = __psp_pa(data);
> +		encrypt->dst_addr =  __sev_page_pa(inpages[0]);
> +	} else {
> +		if (copy_from_user(data, (uint8_t *)debug.src_addr, len)) {
> +			ret = -EFAULT;
> +			goto err_3;
> +		}

Do you need copy_from_user, or can you just pin/unpin memory as for
DEBUG_DECRYPT?

Paolo

> +		d_off = dst_addr & (PAGE_SIZE - 1);
> +		encrypt->length = len;
> +		encrypt->src_addr = __psp_pa(data);
> +		encrypt->dst_addr = __sev_page_pa(inpages[0]);
> +		encrypt->dst_addr += d_off;
> +	}
> +
> +	encrypt->handle = sev_get_handle(kvm);
> +	ret = sev_issue_cmd(kvm, SEV_CMD_DBG_ENCRYPT, encrypt, &argp->error);