On 11/29/21 8:58 AM, Vlastimil Babka wrote:
On 11/29/21 15:44, Brijesh Singh wrote:
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).
AFAIU ptrace() is a scenario where the userspace mapping is being gup-ped,
not a kernel page being kmap()ed?
Yes that is correct.
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.
That could work for the kmap() context.
What to do for the userspace context (host userspace)?
- shared->private transition - page has to be unmapped from all userspace,
elevated refcount (gup() in progress) can block this unmap until it goes
away - could be doable
An unmap of the page from all the userspace process during the page
state transition will be great. If we can somehow store the state
information in the 'struct page' then it can be later used to make
better decision. I am not sure that relying on the elevated refcount is
the correct approach. e.g in the case of encrypted guests, the HV may
pin the page to prevent it from migration.
Thoughts on how you want to approach unmaping the page from userspace
page table?
- still, what to do if host userspace then tries to access the unmapped
page? SIGSEGV instead of SIGBUS and it can recover?
Yes, SIGSEGV makes sense to me.
Thoughts ?
This should turn an RMP fault in the kernel which is not covered in the
uaccess exception table into a fatal error.
Regards,
