Re: [PATCH Part2 RFC v4 10/40] x86/fault: Add support to handle the RMP fault for user address

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux