Messages in this thread | | | Date | Wed, 23 Nov 2022 12:40:40 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH Part2 v6 14/49] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled |
| |
On Tue, Nov 22, 2022 at 05:44:47AM -0600, Kalra, Ashish wrote: > It is important to note that if invalid address/len are supplied, the > failure will happen at the initial stage itself of transitioning these pages > to firmware state.
/me goes and checks out your v6 tree based on 5.18.
Lemme choose one:
static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp) { ...
inpages = sev_pin_memory(kvm, params.uaddr, params.len, &npages, 1);
...
for (i = 0; i < npages; i++) { pfn = page_to_pfn(inpages[i]);
...
ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE, &data, error); if (ret) { /* * If the command failed then need to reclaim the page. */ snp_page_reclaim(pfn);
and here it would leak the pages if it cannot reclaim them.
Now how did you get those?
Through params.uaddr and params.len which come from userspace:
if (copy_from_user(¶ms, (void __user *)(uintptr_t)argp->data, sizeof(params))) return -EFAULT;
Now, think about it, can userspace be trusted?
Exactly.
Yeah, yeah, I see it does is_hva_registered() but userspace can just as well supply the wrong region which fits.
> In such a case the kernel panic is justifiable,
So userspace can supply whatever it wants and you'd panic?
You surely don't mean that.
> but again if incorrect addresses are supplied, the failure will happen > at the initial stage of transitioning these pages to firmware state > and there is no need to reclaim.
See above.
> Or, otherwise dump a warning and let the pages not be freed/returned > back to the page allocator. > > It is either innocent pages or kernel panic or an innocent host > process crash (these are the choices to make).
No, it is make the kernel as resilient as possible. Which means, no panic, add the pages to a not-to-be-used-anymore list and scream loudly with warning messages when it must leak pages so that people can fix the issue.
Ok?
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |