Messages in this thread | | | Date | Thu, 5 Jan 2023 09:37:56 -0600 | Subject | Re: [patch] crypto: ccp - Avoid page allocation failure warning for SEV_GET_ID2 | From | Tom Lendacky <> |
| |
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>
| |