On 12/23/22 06:32, Ilya Leoshkevich wrote:
+ mmap_lock();
+ p = pageflags_find(address, address);
+ mmap_unlock();
How does the code ensure that p is not freed here?
+ return p ? p->flags : 0;
Yep, need to use g_free_rcu.
+ while (true) {
+ PageFlagsNode *p = pageflags_find(start, last);
We can end up here without mmap_lock if we come from the syscall code.
Do we need a retry like in page_get_flags()?
Yep, need to retry.
Speaking of which: does lock_user() actually guarantee that it's safe
to access the respective pages until unlock_user()? If yes, doesn't
this mean that mmap_lock must be held between the two? And if no, and
the SEGV handler is already supposed to gracefully handle SEGVs in
syscall.c, do we need to call access_ok_untagged() there at all?
No, it doesn't, really guarantee anything. We can't hold a lock across a blocking
syscall, so lock_user() can't really lock. We do need to page_unprotect writable pages,
which is the main thing accomplished by the access_ok check.
+ if (qemu_host_page_size <= TARGET_PAGE_SIZE) {
+ start = address & TARGET_PAGE_MASK;
+ len = TARGET_PAGE_SIZE;
+ prot = p->flags | PAGE_WRITE;
+ pageflags_set_clear(start, start + len - 1, PAGE_WRITE, 0);
+ current_tb_invalidated = tb_invalidate_phys_page_unwind(start, pc);
When we come from page_check_range(), pc == 0 and the assertion in
tb_invalidate_phys_page_unwind() fires.
Yep, the assertion is wrong.
Should we pass
current_cpu->cc->get_pc() to page_unprotect() instead of 0, so that
current_tb is resolved to the TB that invoked the syscall?
It's not a guest pc, but the host unwind pc, so, no.
r~
