| Message ID | 20221214202046.719598-1-pgonda@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | crypto: ccp - Limit memory allocation in SEV_GET_ID2 ioctl | expand |
On Wed, 28 Dec 2022, Herbert Xu wrote: > On Tue, Dec 27, 2022 at 05:42:31PM -0800, David Rientjes wrote: > > > > The goal was to be more explicit about that, but setting __GFP_NOWARN > > would result in the same functional behavior. If we're to go that route, > > it would likely be best to add a comment about the limitation. > > > > That said, if AMD would prefer this to be an EINVAL instead of a ENOMEM by > > introducing a more formal limitation on the length that can be used, that > > would be preferred so that we don't need to rely on the page allocator's > > max length to enforce this arbitrarily. > > Ideally the limit should be set according to the object that > you're trying to allocate. But if that is truly unlimited, > and you don't want to see a warning, then GFP_NOWARN seems to > fit the bill. > AMD would be able to speak authoritatively on it, but I think the length of the ID isn't to be assumed by software because it will likely change later. I don't think there's an active vulnerability with the currnet code so we can likely drop stable@vger.kernel.org for this. The kzalloc() will fail if you try to allocate over 2MB. If you try to allocate >32KB, the page allocator will attempt to reclaim but won't oom kill. If you try to allocate <=32KB, there's the potential for oom kill if nothing is reclaimable, but if memory is that scarce I think we have bigger problems. So __GFP_NOWARN will work, but I also think it's subtle enough that it warrants being coupled with a comment.
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 06fc7156c04f..5c16c4406764 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -878,6 +878,10 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp) if (copy_from_user(&input, (void __user *)argp->data, sizeof(input))) return -EFAULT; + /* Max length that can be allocated physically contiguously */ + if (get_order(input.length) >= MAX_ORDER) + return -ENOMEM; + input_address = (void __user *)input.address; if (input.address && input.length) {
Currently userspace can ask for any uint32 size allocation for the SEV_GET_ID2. Limit this allocation size to the max physically contiguously allocation: MAX_ORDER. Reported-by: Andy Nguyen <theflow@google.com> Suggested-by: David Rientjes <rientjes@google.com> Signed-off-by: Peter Gonda <pgonda@google.com> Cc: stable@vger.kernel.org Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: linux-kernel@vger.kernel.org Cc: linux-crypto@vger.kernel.org Cc: John Allen <john.allen@amd.com> Cc: Thomas.Lendacky <thomas.lendacky@amd.com> --- drivers/crypto/ccp/sev-dev.c | 4 ++++ 1 file changed, 4 insertions(+)