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"?
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?
>> 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.
How many more parts of this series are predicated on the behavior of the
VMM like this?
