Re: [PATCH Part2 v5 00/45] Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support

From: Brijesh Singh
Date: Mon Nov 29 2021 - 09:47:05 EST




On 11/25/21 4:05 AM, Joerg Roedel wrote:
On Wed, Nov 24, 2021 at 09:48:14AM -0800, Dave Hansen wrote:
That covers things like copy_from_user(). It does not account for
things where kernel mappings are used, like where a
get_user_pages()/kmap() is in play.

The kmap case is guarded by KVM code, which locks the page first so that
the guest can't change the page state, then checks the page state, and
if it is shared does the kmap and the access.


The KVM use-case is well covered in the series, but I believe Dave is highlighting what if the access happens outside of the KVM driver (such as a ptrace() or others).

One possible approach to fix this is to enlighten the kmap/unmap(). Basically, move the per page locking mechanism used by the KVM in the arch-specific code and have kmap/kunmap() call the arch hooks. The arch hooks will do this:

Before the map, check whether the page is added as a shared in the RMP table. If not shared, then error.
Acquire a per-page map_lock.
Release the per-page map_lock on the kunmap().

The current patch set provides helpers to change the page from private to shared. Enhance the helpers to check for the per-page map_lock, if the map_lock is held then do not allow changing the page from shared to private.

Thoughts ?


This should turn an RMP fault in the kernel which is not covered in the
uaccess exception table into a fatal error.

Regards,