Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <260364a5-f467-f83b-b180-583576ce70ee@amd.com>
Date:   Wed, 4 Jan 2023 08:40:06 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [patch] crypto: ccp - Avoid page allocation failure warning for
 SEV_GET_ID2
Content-Language: en-US
To:     David Rientjes <rientjes@google.com>
Cc:     Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Peter Gonda <pgonda@google.com>,
        Andy Nguyen <theflow@google.com>, linux-kernel@vger.kernel.org,
        linux-crypto@vger.kernel.org, John Allen <john.allen@amd.com>
References: <20221214202046.719598-1-pgonda@google.com>
 <Y5rxd6ZVBqFCBOUT@gondor.apana.org.au>
 <762d33dc-b5fd-d1ef-848c-7de3a6695557@google.com>
 <Y6wDJd3hfztLoCp1@gondor.apana.org.au>
 <826b3dda-5b48-2d42-96b8-c49ccebfdfed@google.com>
 <833b4dd0-7f85-b336-0786-965f3f573f74@google.com>
 <1000d0c8-bd8c-8958-d54f-7e1924fd433d@amd.com>
 <06de8454-2b29-f3b6-7cf2-c037c2735b6d@google.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <06de8454-2b29-f3b6-7cf2-c037c2735b6d@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RXJFSWJONU9jYlkvdnVRTWYyUnZ2Vm0wYzZXL2lwRDFDbnVmQ0VNTlJ4bjI3?=
 =?utf-8?B?eHBnS0lqMnlDbFZiaWg4OUJ2ZDJLYUpMMEtFd0dRc3Q2MkZVMzFRSGJ4S2NY?=
 =?utf-8?B?bklNQ21MbzA0cHoyd0loVWZjRkdDcURHRHBzVytmbm1MSDhteW1JakNnTHZY?=
 =?utf-8?B?SytRU3FMR0h4ZHZzZEF5K0xWY0ZuemptSGFRend0ekV6VDRySEZkZDhEdVUv?=
 =?utf-8?B?SkQyeHRkREErNm1Rc0pMUUdPdW0rMDVmY1J4MEtObGd6V1BiaEMwdy9PV0FT?=
 =?utf-8?B?ZzdGUDUyTUNRZys0c2hVc1pnZXlTVkJBOWJyM1V3MFJ4UEVJa2ZKMDY1WTJh?=
 =?utf-8?B?SlpwQjJsUUtCSlJQVGpqRW9oUU5odEo3ZUdQcUZYQXgvRGdTdHJPdTJ3a2w3?=
 =?utf-8?B?OHBqOTV5U2xHZzJHM3RyS0QzMEs2OC9PeGRWYWc4ci9HZENCb2JJaCtGRkUr?=
 =?utf-8?B?Mjd1NEFJM3RnNUFEMWd2UE0yalIzMmRPZS85cnNLWDczYmlvSTZ0dDl0dzFO?=
 =?utf-8?B?TTNiS0VqTVFiWDhXWGd6Um5uTnQxNTEyTFVhL2JNQURJbEkwRUVlVEJJRUxM?=
 =?utf-8?B?TWQydm5Ba2k5a0VPOC9zUG1kZzdDd0VYTVdyRWtnYmJPRUR0WVNmN1R4WExE?=
 =?utf-8?B?QXdiSDdwWTY3djRTQzhjOXFHRXZzSlFLR1RDcU5JNlZxS0w3a3drbmVERU9N?=
 =?utf-8?B?T2F3MkJrcE9kekUrWkRqdmttZEoycFhBZ1pVb1ZOckNCOE42cHZFQmVDVHdK?=
 =?utf-8?B?N1JzUEFFZ3RqVTNIMEhmQW9wMDcySm9FTEV5elJ4WjZSTjFLTXNPdmp4QzY4?=
 =?utf-8?B?M0k1TTRaZk5FLzBBMUhDcmNBL2k2OElvd2YvaVJvWmxja0RNMER5aWFrUndD?=
 =?utf-8?B?RS8wbmQ5QmJXekF0UkZpNDFsOUdORjZ5eHRKZTFCUjZ1eFU4b0xKL3JOMnJn?=
 =?utf-8?B?ajAzM2VsbXhDM1JiODFBdERhRUQ3Q0g4U1RmT2NMbXQ4K0prTXBvMnM0YS9C?=
 =?utf-8?B?cTRMeTZqNWwyNlFhN3pJbnFrUTQzMUJsZ1dpaXZGUkxzM1ZQdjBGWXZwTXlO?=
 =?utf-8?B?MkdxVmQ4REJaNWlMNGliM2hNRUJxTmR6SWF3ZkdFMHI4dStNaFNYWloyRllw?=
 =?utf-8?B?Zm0yc2JrN0Y5VFRTNXd5YllMaHlJOTZ4ODNseGdQRGlQRy9ZVUxaZGVweVlt?=
 =?utf-8?B?NGZFUWJrbXIrYm9wWWpuZFM1d29QM1FPdDQ1ekgwbnlBNUZnbjhRRWdndWZp?=
 =?utf-8?B?MlJFd0IzU2h5cFRqUFhRY0tpRDVyY1hMZlhrNWhSWVVtZ0FqTXpWNUxxR3A1?=
 =?utf-8?B?bi95bzdOUjlvZ0hRaVdwdzRsbkVaUEREZlVWWEY1R3hJVGhTUTVNUzZJdEk1?=
 =?utf-8?B?MXhOY1pIbjRnc21rYzJGZjBQZjZlRTBHWnIwd0dwTW90dXFwWkh0QnpuSDZy?=
 =?utf-8?B?NnhEcXc1MU5TYW5maXdKV0FhcFZFaVZvR2kzRXNWSXhuZ2Y5NEQxQVRjQWdl?=
 =?utf-8?B?Y01BeTAyak5wekRkWTlGRVVWOXFrTmVHKzc3Vmk1WlNSOVNHcVU5L0U3dG0r?=
 =?utf-8?B?OXRVMEZua2pNaDdOMW5BM0N5MzN1NDJwMTZsSTNiRXYvTk9SYXlvTGxxNTl3?=
 =?utf-8?B?TWdzWW1ZdG5kR2FTRko0UGZDS2RyUG5WTm1FY3RwakV2UnRFeCt5Y1o4MWpl?=
 =?utf-8?B?U3NkeEJoa0pjdlVBajZRSHoyM2VZblJjdnVjQzJmcElqR2oycjFUY2MxM2RI?=
 =?utf-8?B?bzZqcFVBZVk5Tzhhd090QUR1RjlNV1IrUVk3NTVnbXQrNjFRQjQwYzNTTjRu?=
 =?utf-8?B?SWlYeExIbVBCaElIYjlNRVZRMXczYmx5VUlXd1JINFZET2lCNXhpNXp6ZXVB?=
 =?utf-8?B?ZHNWQXFRRThvK2M2TkJuU1BxZmNmekwvUUxJR3poU1RGQ2lZRU1nVXQrSkVJ?=
 =?utf-8?B?SWZJNTd3ZTU4cVZ1cUZxUzJlUjMrLzFTU21DZ09KQms2MkFuWkVHSWhwc29E?=
 =?utf-8?B?TDdNZUpPQWFZOE5NS3FoTUpSdnZhdjhMK2pKN0xPMjMwNmJTSzcrTDBSeHdZ?=
 =?utf-8?B?YU1rK2lQMkpLdndIckJ3SmpNT3NqTnNsbG9WUEdIb2lOdFFSUis3V3VLQ3pn?=
 =?utf-8?Q?S1oubed4oHQ9/feYggmgm/iNt?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 795f1b70-d7c7-4ad5-4d79-08daee6193b6
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2023 14:40:08.5032
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 56ybnUT/tkunGN8A7CQqiXhqnaDQiuR/RJn2IsrgMhWvxk7E4tm5GvyQ0nUz7cOf5FWAMnrJK53ZXeV9VqrP8A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB6224
X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 1/3/23 17:18, David Rientjes wrote:
> On Tue, 3 Jan 2023, Tom Lendacky wrote:
> 
>> On 12/30/22 16:18, David Rientjes wrote:
>>> For SEV_GET_ID2, the user provided length does not have a specified
>>> limitation because the length of the ID may change in the future.  The
>>> kernel memory allocation, however, is implicitly limited to 4MB on x86 by
>>> the page allocator, otherwise the kzalloc() will fail.
>>>
>>> When this happens, it is best not to spam the kernel log with the warning.
>>> Simply fail the allocation and return ENOMEM to the user.
>>>
>>> Fixes: d6112ea0cb34 ("crypto: ccp - introduce SEV_GET_ID2 command")
>>> Reported-by: Andy Nguyen <theflow@google.com>
>>> Reported-by: Peter Gonda <pgonda@google.com>
>>> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
>>> Signed-off-by: David Rientjes <rientjes@google.com>
>>> ---
>>>    drivers/crypto/ccp/sev-dev.c | 9 ++++++++-
>>>    1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>>> --- a/drivers/crypto/ccp/sev-dev.c
>>> +++ b/drivers/crypto/ccp/sev-dev.c
>>> @@ -881,7 +881,14 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd
>>> *argp)
>>>    	input_address = (void __user *)input.address;
>>>      	if (input.address && input.length) {
>>> -		id_blob = kzalloc(input.length, GFP_KERNEL);
>>> +		/*
>>> +		 * The length of the ID shouldn't be assumed by software since
>>> +		 * it may change in the future.  The allocation size is
>>> limited
>>> +		 * to 1 << (PAGE_SHIFT + MAX_ORDER - 1) by the page allocator.
>>> +		 * If the allocation fails, simply return ENOMEM rather than
>>> +		 * warning in the kernel log.
>>> +		 */
>>> +		id_blob = kzalloc(input.length, GFP_KERNEL | __GFP_NOWARN);
>>
>> We could do this or we could have the driver invoke the API with a zero length
>> to get the minimum buffer size needed for the call. The driver could then
>> perform some validation checks comparing the supplied input.length to the
>> returned length. If the driver can proceed, then if input.length is exactly 2x
>> the minimum length, then kzalloc the 2 * minimum length, otherwise kzalloc the
>> minimum length. This is a bit more complicated, though, compared to this fix.
>>
> 
> Thanks Tom.  IIUC, this could be useful to identify situations where
> input.length != min_length and input.length != min_length*2 and, in those
> cases, return EINVAL?  Or are there situations where this is actually a
> valid input.length?
> 
> I was assuming that the user was always doing its own SEV_GET_ID2 first to
> determine the length and then use it for input.length, but perhaps that's
> not the case and they are passing a bogus value.

Except that if the user was always doing that, then we wouldn't be worried 
about this case then. But, I think my method is overkill and the simple 
approach of this patch is the way to go.

Thanks,
Tom