qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH RFC] linux-user: Allow gdbstub to ignore page protection


From: Richard Henderson
Subject: Re: [PATCH RFC] linux-user: Allow gdbstub to ignore page protection
Date: Thu, 21 Dec 2023 10:33:51 +1100
User-agent: Mozilla Thunderbird

On 12/16/23 10:24, Ilya Leoshkevich wrote:
@@ -377,22 +379,42 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
          flags = page_get_flags(page);
          if (!(flags & PAGE_VALID))
              return -1;
+        prot = ((flags & PAGE_READ) ? PROT_READ : 0) |
+               ((flags & PAGE_WRITE) ? PROT_WRITE : 0) |
+               ((flags & PAGE_EXEC) ? PROT_EXEC : 0);

See target_to_host_prot where guest PROT_EXEC is mapped to host PROT_READ.
This should be

        flags & (PAGE_READ | PAGE_EXEC) ? PROT_READ

+            if (!(prot & PROT_WRITE)) {
+                if (target_mprotect(page, TARGET_PAGE_SIZE,
+                                    prot | PROT_WRITE)) {
+                    return -1;
+                }
+            }
             /* XXX: this code should not depend on lock_user */
-            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
+            if (!(p = lock_user(VERIFY_NONE, addr, l, 0)))
                 return -1;
             memcpy(p, buf, l);
             unlock_user(p, addr, l);
+            if (!(prot & PROT_WRITE)) {
+                if (target_mprotect(page, TARGET_PAGE_SIZE, prot)) {
+                    return -1;
+                }
+            }

Is the mmap lock held here? You're opening up a window in which a page may be modified but we don't catch the modification to translation blocks.

+            if (!(prot & PROT_READ)) {
+                if (target_mprotect(page, TARGET_PAGE_SIZE, prot | PROT_READ)) 
{
+                    return -1;
+                }
+            }

I really really doubt you need to do this. The page is not accessible, so why expose it at all? If you want to do this, I wonder if we should instead be interacting with ptrace?


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]