From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1758089AbdJMJPG (ORCPT <rfc822;w@1wt.eu>);
        Fri, 13 Oct 2017 05:15:06 -0400
Received: from mx2.suse.de ([195.135.220.15]:33364 "EHLO mx2.suse.de"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751680AbdJMJO4 (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 13 Oct 2017 05:14:56 -0400
Date: Fri, 13 Oct 2017 11:14:37 +0200
From: Borislav Petkov <bp@suse.de>
To: Brijesh Singh <brijesh.singh@amd.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
        Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Gary Hook <gary.hook@amd.com>, Tom Lendacky <thomas.lendacky@amd.com>,
        linux-crypto@vger.kernel.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [Part2 PATCH v5.1 12.7/31] crypto: ccp: Implement SEV_PEK_CSR
 ioctl command
Message-ID: <20171013091437.izbdktceyam54v3q@pd.tnic>
References: <20171004131412.13038-13-brijesh.singh@amd.com>
 <20171007010607.78088-1-brijesh.singh@amd.com>
 <20171007010607.78088-7-brijesh.singh@amd.com>
 <20171012195331.bdzwqzyrjc6fi5lj@pd.tnic>
 <d83b3e27-1195-817e-e976-36982e275588@amd.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <d83b3e27-1195-817e-e976-36982e275588@amd.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Oct 12, 2017 at 09:24:01PM -0500, Brijesh Singh wrote:
> I assume you mean performing the SEV state check before allocating the
> memory for the CSR blob, right ?

I mean, do those first:

        if (copy_from_user(&input, (void __user *)argp->data, sizeof(input)))
                return -EFAULT;

        if (!input.address)
                return -EINVAL;

        /* allocate a physically contiguous buffer to store the CSR blob */
        if (!access_ok(VERIFY_WRITE, input.address, input.length) ||
            input.length > SEV_FW_BLOB_MAX_SIZE)
                return -EFAULT;


Because if you allocate the memory first and some of those checks fail,
you allocate in vain to free it immediately after. And you can save
yourself all that.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--