From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753599AbdJMEN5 (ORCPT ); Fri, 13 Oct 2017 00:13:57 -0400 Received: from mail-dm3nam03on0080.outbound.protection.outlook.com ([104.47.41.80]:44546 "EHLO NAM03-DM3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752612AbdJMENy (ORCPT ); Fri, 13 Oct 2017 00:13:54 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=brijesh.singh@amd.com; Cc: brijesh.singh@amd.com, Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Herbert Xu , Gary Hook , Tom Lendacky , 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 To: Borislav Petkov 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> From: Brijesh Singh Message-ID: Date: Thu, 12 Oct 2017 23:13:44 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: CY4PR02CA0041.namprd02.prod.outlook.com (10.175.57.155) To SN1PR12MB0159.namprd12.prod.outlook.com (10.162.3.146) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 2cccf52a-3541-45a7-7a66-08d511f0cee2 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(2017030254152)(48565401081)(2017052603199)(201703131423075)(201703031133081)(201702281549075);SRVR:SN1PR12MB0159; X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0159;3:K/5x+d7ts0WwWCV4d6zDc1QzErEGHmK+5E3aKgVo3exFbX8RAWTYYdmeiicpq1pDIHfeOvbS5pWB5uzhauWxT/t4jW8fDdX+v+H+GRWE5oqjsGavEP++IA19p6XRzUrnwkQiCcUOnZ0d+ngRCwmBQ8fub5eTIheouzHxDSK2T9NAQctpYCD9KhuOBd2c2KCl/UyD5KmhisZSnabSGlSrTGdr6m+jWNC6srTGuod8Kmu8kQOdafszUuGz6cbigU7a;25:z5crISRaqog+wLrORFVi9lHbuYqal5R7TkxETKgL+R0pZsx5zyuvVjjqF0lAwW4IuS4cQn5SC/Mzl7+LlMVAL1/+X+L4e6Y+EwOjBjl2ysJhmQkQBUpKVQKahedzZbyyCD5OfkaWsRDbYZ9PxFmHwiVBtJurmpe9e5OaWMaDGWlxe2xWs3tDIffpEo4h9MF4h6WALvjTlwLM7Y1ANBCRmgkCsSH6MsWFfGdT/OR0c3YUFJ+nH1rA0gziRSYMvn1Dnh9sLhEjKnMVlqoSOMpZhRUAlvNBICrZX7GyEiZoj+rwj4tii6VnHenfy9t1XoCYK3RtA8MK6Ete6AIppKYIkw==;31:16G0h7izmCzJd+AFhexhNbNuGYke55NYTydLokwTbNBzdJy/4D8YsrUzExMR8eAt3HGtj+loiDN6vDKsGUr9uYtl5kVxhoZCtOOsRf41xsPb7WIdNXmJQJnsYxSNC6fG96ax9QoAjBZ2ECC3IhBc/CwZvYnmhuRnIyxtdMiJndG4+uf4oa03Rg21F5NOWCiKXZ0wfr6E8MRlRyOEHPG11+A0cX5nbOL42urcJkDDFnc= X-MS-TrafficTypeDiagnostic: SN1PR12MB0159: X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0159;20:z0rkCHYz5nYX1axoH9nEWa0F+YfPmh1dttMYZeMxjlib8s+W66fsQELUyMXiqBRRkHnkjCqUfTDbYcab9oiCHwDXhnbAEQo+6OHrvAzXNbI5kgofOWtc6I2uGqGRtNOuLJ4TvIVg/1XQRrED9JQeLK6oWivQUp17k5r8VwyeBPlm0fX5kClqn6DOU03MNOWGI33X5ze4xkX5orQ7ATMkTNOGa+nkqu/tzFxShPWu9coJFc7GCiUUx6jaO/O98nzoCvvM8qj5VNL6cqoXNO9K+whdScQZjH4a35L03tsGbcuTpJ3EUNE/c4+xAe3l3jVK/rf7DYiNlXyLPWlOU3tsJ4LTZmUy5WyGbbCvmBYiHyh1GGH+eb3eKh/ry53sPeU45NACY4btv6Fgd+hDcJxxJ5xL3eaU0UdEpxvMMQcjzA75upb1kRr5pK+Hp6rpvqjOLOCWAxD++7XwAAhRyM5zwukJd43iCdhNRnbsOyA3jG1vBnh8UrgIx3VAk+6DGDR8;4:91Nz0qZeVh0+LxSoI4M9ZrpGMDuTHhUBk7nV7/J9sbF97DQ7W9/bTN8cZ0VzWC5s5hH/eVZYQEnV9XZJVpRM8wzEUv17MM/a9kOKNbUoKwGFxvli1ldd9ScpdDS/c0oVatFjPDRXjflSSH+CXpC+UYLhAMYB2laeDpzSJKj7HPx6Kr38cCCvkGNp2ZOyaKaVfUn8JaIHJjHNRpY2a2MpkRRSGCDgw8L2eHP4KwCJnvewOWRpMqPWOFYRNaLPsIhY X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(8121501046)(5005006)(93006095)(93001095)(10201501046)(100000703101)(100105400095)(3002001)(6055026)(6041248)(20161123558100)(20161123564025)(20161123562025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:SN1PR12MB0159;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:SN1PR12MB0159; X-Forefront-PRVS: 04599F3534 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(39860400002)(376002)(346002)(377454003)(189002)(5423002)(24454002)(199003)(6116002)(8936002)(16526018)(189998001)(5660300001)(8676002)(83506001)(3846002)(4326008)(65806001)(64126003)(2906002)(53546010)(2870700001)(6246003)(23676002)(50466002)(68736007)(65956001)(81166006)(66066001)(2950100002)(54906003)(6666003)(81156014)(316002)(6916009)(58126008)(31686004)(86362001)(6486002)(31696002)(33646002)(478600001)(93886005)(50986999)(36756003)(97736004)(76176999)(54356999)(229853002)(25786009)(47776003)(53936002)(53416004)(305945005)(105586002)(7736002)(65826007)(101416001)(106356001);DIR:OUT;SFP:1101;SCL:1;SRVR:SN1PR12MB0159;H:wsp094129wss.amd.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtTTjFQUjEyTUIwMTU5OzIzOnhEM21QaHFRYTh0dk80cFMremlZMm5rQU9k?= =?utf-8?B?dkQvTUs0WWlIZ3c3MW9LcVJLQWRHbEdVSXVHRWkvR3hFLyt3QTJUdDJnZFdk?= =?utf-8?B?eUVvYWpMUmR3c3FOVHNFYkhJRnpTNlhzc3lRU2VXWU84N3FKaWkvRVgrNWRI?= =?utf-8?B?TWtiZ3JiWGlzL1NyVTJ2ZnhZOG1jM1A3YjZyWkVqb1RobXlZc052di8xNkRr?= =?utf-8?B?ZlM1UTFjQ1o5SnZYTzZlY095RkMzYnZrNENFazVQbEdQWjgyTkM2bjFiRnR3?= =?utf-8?B?RUlkZDFzTENWbzZML0hra0Z2Zkh1d1MvdE5iNzJEem9ITmU5V1pFU3JlbnJO?= =?utf-8?B?bkVHTStzZkNOcjFtbHVTV3pvdEErU21DT3NXZ09iejlwSVJXNTFsZHZJYnNi?= =?utf-8?B?TndMcjJiVUpZWGZOSWswZmhEa1liRU1naDM4dy9LUGx4SnpwdGYxVmZOQ0Yy?= =?utf-8?B?NERKbVo5WEx2RXJPSUdVWUFYYUdZR2NhTnM0WGJlbWJnV2JyOURrT2piUUNy?= =?utf-8?B?OG1YVjJaZHFqY0dSSWVQVytQUWVxWW15N0ovVnMxRmZsYmcvSHZwNW1RK1Fi?= =?utf-8?B?UnIzQkNaTlNwUHFlYk9yRXA5WmM0TE1QZDZ4N01jQk9hTjZjclhzR2ZVc1N1?= =?utf-8?B?VEJRYUQ1TVgwRXd3dWpUVTVyUERpM0N6R3d0MnRZSFVpTURJOGJ4cUNhTXpt?= =?utf-8?B?VVRKUDdDdkgyQ3paOXpuVVVYb2lYeGtxS2dEWmkxQzdRYWRKVU9vQUtDNW4r?= =?utf-8?B?d0VJS0U5eExPUTJZN2pRS011ZXVVYnQxUDJhSGNtTDdCLzg2Vlk0RisrcFQy?= =?utf-8?B?Kzdwcm9jSkxUMTU1TG5SMUYxS3NueFhnR25xTVFKTUt6cHF1ZXowa3JMMGMw?= =?utf-8?B?c0gzU2dxNmVNTENlNnQ0ckZZMThRV1A0amk1dnEyeGxaQTh0UFpDUWNYQXJO?= =?utf-8?B?WFZFMUR4Q0tVTWxPZlZkUjcvNERlWUNlRmN4QXd0UG1JV3ZxYXd2SEN4eFhO?= =?utf-8?B?eVJDdGVtTlJXQkRmUkdacDFKdDlQUjBIQi92dlhaald5bE1NSVdxTWswcndx?= =?utf-8?B?L01raUF3cmRuVml1VENjUDgwUGgzOEhIY2w3d2dTcW5TbGUrb3JhYVFndG9R?= =?utf-8?B?Mm5wNGtnS0RMQm9pNThtc3IxNTRNbGtwc0F5dGtFMG8vWHVkbDMvV0xMby93?= =?utf-8?B?MXVjSXNpaHkyV1hzakpUR0NuNDV1Z0k2UnB1YXFQUHdCbXZzeVBnM2xYS21K?= =?utf-8?B?R1lTYk9oTU9qUGg0WkpEeW4rMDJ5TnlBb2o4ejd4blZma0JaUUIwMUNNTFNU?= =?utf-8?B?amppOEM0UVR5MjJjRHB1OW9IVks4REFyMVFTM2hEMHFGUjMvZUsyR2RxMk1U?= =?utf-8?B?UXpzRzBtWXEwSUFHWm1pdXJHcGRCRVBnc05oK3cvVXE5YWY3OG1ZeU5HY1RP?= =?utf-8?B?M2ZaLzVCdDhkK2VHWEpPRjlGVkMvbUpKUzdGVFhyd3RQbDNjZmFONS8zU1BD?= =?utf-8?B?cUZxcGltZEFHT3Q0emNnMFpNK3BZTmhqQXZSYzlJWjNublNUcGhNb0Jka3Ez?= =?utf-8?B?VUl4TUR0TDhsb2FwTmtHSjJHZ1FWQ05mU3RsMXprWW5laHByZ0FjQ1JLVEtY?= =?utf-8?B?L3hBblZTSnFFb3gxSmpVZEZybEpNVGc4LytERVgyWFA4Uy9YR1RmNmtjK2tj?= =?utf-8?B?eklKWmk4ajZXTHM5cU5TQmtkVEhJM2MvdGY3VWtDalg5bHJVQTBNNWloRCtm?= =?utf-8?B?ZTY0RnJyYUx4ajM3dkpKRzc0RFM1Y0g1Y2M5WGFiOVJvd2p5NnB3ZHY5WTYr?= =?utf-8?B?T243WXhYVEhGeDY0U0ovUnMvOFMyK0xhVWpXaUVmVFhCbWt6ZC9tTDBleSt6?= =?utf-8?Q?8WSUKqmLZcb3m2DS+XC1TMJUleDq07B2?= X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0159;6:1Zzj5kQsRukDCnoZHXRNibmR9nXu67KDeLabEVqRjWZJZbJotwSNKIqDMGSR/uFWvVecheA50txpYPHowYehtx4uzty6aAAl8YaLtxHkYDaHxAHxU7lI7iFTfr5r4S9o1CUl4skEkx0vcyJlUMamuqY7AWIh845BCPF1rvPs8Zvo64gSivgQkTmxDOuS3KSs/C8X85xrsEGmO1TmrxFP3XwB0Z3AWqYpiPqDWuwMMp5KhW5TDiVRjf8vE8Oi/GQYxPBr+gFb4icHDruCkSTmxqmsb+PFD4zYDxGRrI8YEAwBT0urmC2CmmDbWIQGhNAeTV30p/hFjhcvg1QjM45jVA==;5:dY4scvTafeNWilomnu6YNQHcQvZ3UQoJ466t1hqvajCQzyFPyFJlJoLjHpRBH5y64peGKoToZ1oYLxYccumx+mcxqK7JJw//xNgX/RezbS4IoLHpW5aVpf2bZU0vqV+3q6/qvJD3FvoGG/DG0jUX6OkcizMYWjBAJ57ButNWA4o=;24:UrLzRnvX5WbGDVriPClWg6zFMUZnW3ZODLr7eVCd1fEZSI1HTwWd9HU/Dzo+9PG5PknxlhqroLyeDz6GhlvHWvEy2sXPgw3pYzZnSL4aEt4=;7:APR4QVXOM9Q1peD+DsuieHQYuzxmXr9ttvQqCmKFsFbPLFH7gnI5WpWEaxW8+wKBlGJC6ADTnWQ/ksEECh4C2RZOS1Ckv1GXfdjYq79pTl4ltrBxPiXDaJuKD4ORMvkoxOccCkfmE4iKTZpzui80JWUSBxTj5TrOWogymzZiO3pa7viDW76ncEpxJCdMSs5j210bf8xNvjNy+fpNEP9XqgYppHgJbTSx0zXbM9KzlDM= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;SN1PR12MB0159;20:0Mfg5PuhUAH1LJeh2GfW3d9jVZIsNZB4IRZDuCdwFtdZeX8zZaJyJ97+KoX9ipxUljVht+LktkYVBGWfs22LAkCs/M+XJ8rq7Jkv1ObljTKjN8Vu52Mixcl9ZedBSr6ZKy2cj1HtnUlNnPpdMJ+6mwSlUPOat7DP9PVElW0qPnky/LFcAxPi8TKxALscGH3s3zHU9qfRf3Qt5/zh+oZrZmRhKPJz7MPtDAbglanybJj2zjNPuNaI45WxEyjMrEl4 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2017 04:13:49.2159 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0159 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/12/17 9:24 PM, Brijesh Singh wrote: > > On 10/12/17 2:53 PM, Borislav Petkov wrote: > ... > >> Ok, a couple of things here: >> >> * Move the checks first and the allocations second so that you allocate >> memory only after all checks have been passed and you don't allocate >> pointlessly. > > I assume you mean performing the SEV state check before allocating the > memory for the CSR blob, right ? In my patches, I typically perform all > the SW specific checks and allocation before invoking the HW routines. > Handling the PSP commands will take longer compare to kmalloc() or > access_ok() etc. If its not a big deal then I would prefer to keep that > way. > > >> * That: >> >> if (state == SEV_STATE_WORKING) { >> ret = -EBUSY; >> goto e_free_blob; >> } else if (state == SEV_STATE_UNINIT) { >> ret = sev_firmware_init(&argp->error); >> if (ret) >> goto e_free_blob; >> do_shutdown = 1; >> } >> >> is a repeating pattern. Perhaps it should be called >> sev_firmware_reinit() and called by other functions. > >> * The rest is simplifications and streamlining. >> >> --- >> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c >> index e3ee68afd068..d41f5448a25b 100644 >> --- a/drivers/crypto/ccp/psp-dev.c >> +++ b/drivers/crypto/ccp/psp-dev.c >> @@ -302,33 +302,30 @@ static int sev_ioctl_pek_csr(struct sev_issue_cmd *argp) >> int ret, state; >> void *blob; >> >> - if (copy_from_user(&input, (void __user *)(uintptr_t)argp->data, >> - sizeof(struct sev_user_data_pek_csr))) >> + if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) >> + return -EFAULT; >> + >> + if (!input.address) >> + return -EINVAL; >> + As per the spec, its perfectly legal to pass input.address = 0x0 and input.length = 0x0. If userspace wants to query CSR length then it will fill all the fields with. In response, FW will return error (LENGTH_TO_SMALL) and input.length will be filled with the expected length. Several command work very similar (e.g PEK_CSR, PEK_CERT_EXPORT). A typical usage from userspace will be: - query the length of the blob (call command with all fields set to zero) - SEV FW will response with expected length - userspace allocate the blob and retries the command.  >> + /* 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; >> >> data = kzalloc(sizeof(*data), GFP_KERNEL); >> if (!data) >> return -ENOMEM; >> >> - /* allocate a temporary physical contigous buffer to store the CSR blob */ >> - blob = NULL; >> - if (input.address) { >> - if (!access_ok(VERIFY_WRITE, input.address, input.length) || >> - input.length > SEV_FW_BLOB_MAX_SIZE) { >> - ret = -EFAULT; >> - goto e_free; >> - } >> - >> - blob = kmalloc(input.length, GFP_KERNEL); >> - if (!blob) { >> - ret = -ENOMEM; >> - goto e_free; >> - } >> - >> - data->address = __psp_pa(blob); >> - data->len = input.length; >> + blob = kmalloc(input.length, GFP_KERNEL); >> + if (!blob) { >> + ret = -ENOMEM; >> + goto e_free; >> } >> >> + data->address = __psp_pa(blob); >> + data->len = input.length; >> + >> ret = sev_platform_get_state(&state, &argp->error); >> if (ret) >> goto e_free_blob; >> @@ -349,25 +346,23 @@ static int sev_ioctl_pek_csr(struct sev_issue_cmd *argp) >> do_shutdown = 1; >> } >> >> - ret = sev_handle_cmd(SEV_CMD_PEK_CSR, data, &argp->error); >> + ret = sev_do_cmd(SEV_CMD_PEK_CSR, data, &argp->error); >> >> input.length = data->len; >> >> /* copy blob to userspace */ >> - if (blob && >> - copy_to_user((void __user *)(uintptr_t)input.address, >> - blob, input.length)) { >> + if (copy_to_user((void __user *)input.address, blob, input.length)) { >> ret = -EFAULT; >> goto e_shutdown; >> } >> >> - if (copy_to_user((void __user *)(uintptr_t)argp->data, &input, >> - sizeof(struct sev_user_data_pek_csr))) >> + if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) >> ret = -EFAULT; >> >> e_shutdown: >> if (do_shutdown) >> - sev_handle_cmd(SEV_CMD_SHUTDOWN, 0, NULL); >> + ret = sev_do_cmd(SEV_CMD_SHUTDOWN, 0, NULL); >> + >> e_free_blob: >> kfree(blob); >> e_free: >> @@ -408,10 +403,10 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg) >> ret = sev_ioctl_pdh_gen(&input); >> break; >> >> - case SEV_PEK_CSR: { >> + case SEV_PEK_CSR: >> ret = sev_ioctl_pek_csr(&input); >> break; >> - } >> + >> default: >> ret = -EINVAL; >> goto out; >>