[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <06de8454-2b29-f3b6-7cf2-c037c2735b6d@google.com>
Date: Tue, 3 Jan 2023 15:18:31 -0800 (PST)
From: David Rientjes <rientjes@...gle.com>
To: Tom Lendacky <thomas.lendacky@....com>
cc: Herbert Xu <herbert@...dor.apana.org.au>,
"David S. Miller" <davem@...emloft.net>,
Peter Gonda <pgonda@...gle.com>,
Andy Nguyen <theflow@...gle.com>, linux-kernel@...r.kernel.org,
linux-crypto@...r.kernel.org, John Allen <john.allen@....com>
Subject: Re: [patch] crypto: ccp - Avoid page allocation failure warning for
SEV_GET_ID2
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@...gle.com>
> > Reported-by: Peter Gonda <pgonda@...gle.com>
> > Suggested-by: Herbert Xu <herbert@...dor.apana.org.au>
> > Signed-off-by: David Rientjes <rientjes@...gle.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.
Powered by blists - more mailing lists