Messages in this thread | | | Subject | Re: [PATCH Part2 RFC v4 10/40] x86/fault: Add support to handle the RMP fault for user address | From | Brijesh Singh <> | Date | Mon, 12 Jul 2021 11:11:54 -0500 |
| |
On 7/12/21 11:00 AM, Dave Hansen wrote: > On 7/12/21 8:43 AM, Brijesh Singh wrote: >>>> + /* >>>> + * The backing page level is higher than the RMP page level, >>>> request >>>> + * to split the page. >>>> + */ >>>> + if (level > rmp_level) >>>> + return RMP_FAULT_PAGE_SPLIT; >>> >>> This can theoretically trigger on a hugetlbfs page. Right? >> >> Yes, theoretically. >> >> In the current implementation, the VMM is enlightened to not use the >> hugetlbfs for backing page when creating the SEV-SNP guests. > > "The VMM"?
I was meaning a userspace qemu.
> > We try to write kernel code so that it "works" and doesn't do unexpected > things with whatever userspace might throw at it. This seems to be > written with an assumption that no VMM will ever use hugetlbfs with SEV-SNP. > > That worries me. Not only because someone is sure to try it, but it's > the kind of assumption that an attacker or a fuzzer might try. > > Could you please test this kernel code in practice with hugetblfs?
Yes, I will make sure that hugetlbfs path is tested in non-RFC version.
> >>> I also suspect you can just set VM_FAULT_SIGBUS and let the do_sigbus() >>> call later on in the function do its work. >>>> +static int handle_split_page_fault(struct vm_fault *vmf) >>>> +{ >>>> + if (!IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) >>>> + return VM_FAULT_SIGBUS; >>>> + >>>> + __split_huge_pmd(vmf->vma, vmf->pmd, vmf->address, false, NULL); >>>> + return 0; >>>> +} >>> >>> What will this do when you hand it a hugetlbfs page? >> >> VMM is updated to not use the hugetlbfs when creating SEV-SNP guests. >> So, we should not run into it. > > Please fix this code to handle hugetlbfs along with any other non-THP > source of level>0 mappings. DAX comes to mind. "Handle" can mean > rejecting these. You don't have to find some way to split them and make > the VM work, just fail safely, ideally as early as possible. > > To me, this is a fundamental requirement before this code can be accepted.
Understood, if userspace decided to use the hugetlbfs backing pages then I believe earliest we can detect is when we go about adding the pages in the RMP table. I'll add a check, and fail the page state change.
-Brijesh
| |