lkml.org 
[lkml]   [2023]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [patch] crypto: ccp - Avoid page allocation failure warning for SEV_GET_ID2
From
On 1/4/23 19:49, David Rientjes wrote:
> On Wed, 4 Jan 2023, Tom Lendacky 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.
>>
>
> Makes sense, thanks for the clarification. Does that translate into an
> acked-by? :)

Ah, yeah, forgot about that!

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

\
 
 \ /
  Last update: 2023-03-26 23:28    [W:0.054 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site