lkml.org 
[lkml]   [2021]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH Part2 RFC v4 10/40] x86/fault: Add support to handle the RMP fault for user address
From
Date


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

\
 
 \ /
  Last update: 2021-07-12 18:13    [W:0.142 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site