gdbserver ignores page protection by virtue of using /proc/$pid/mem.
Teach qemu gdbstub to do this too. This will not work if /proc is not
mounted; accept this limitation.
One alternative is to temporarily grant the missing PROT_* bit, but
this is inherently racy. Another alternative is self-debugging with
ptrace(POKE), which will break if QEMU itself is being debugged - a
much more severe limitation.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
cpu-target.c | 55 ++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/cpu-target.c b/cpu-target.c
index 5eecd7ea2d7..69e97f78980 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -406,6 +406,15 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
vaddr l, page;
void * p;
uint8_t *buf = ptr;
+ int ret = -1;
+ int mem_fd;
+
+ /*
+ * Try ptrace first. If /proc is not mounted or if there is a different
+ * problem, fall back to the manual page access. Note that, unlike ptrace,
+ * it will not be able to ignore the protection bits.
+ */
+ mem_fd = open("/proc/self/mem", is_write ? O_WRONLY : O_RDONLY);
while (len > 0) {
page = addr & TARGET_PAGE_MASK;
@@ -413,22 +422,33 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
if (l > len)
l = len;
flags = page_get_flags(page);
- if (!(flags & PAGE_VALID))
- return -1;
+ if (!(flags & PAGE_VALID)) {
+ goto out_close;
+ }
if (is_write) {
- if (!(flags & PAGE_WRITE))
- return -1;
+ if (mem_fd == -1 ||
+ pwrite(mem_fd, ptr, len, (off_t)g2h_untagged(addr)) != len) {
+ if (!(flags & PAGE_WRITE)) {
+ goto out_close;
+ }
+ /* XXX: this code should not depend on lock_user */
+ p = lock_user(VERIFY_WRITE, addr, l, 0);
+ if (!p) {
+ goto out_close;
+ }
+ memcpy(p, buf, l);
+ unlock_user(p, addr, l);
+ }
+ } else if (mem_fd == -1 ||
+ pread(mem_fd, ptr, len, (off_t)g2h_untagged(addr)) != len) {
+ if (!(flags & PAGE_READ)) {
+ goto out_close;
+ }
/* XXX: this code should not depend on lock_user */
- if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
- return -1;
- memcpy(p, buf, l);
- unlock_user(p, addr, l);
- } else {
- if (!(flags & PAGE_READ))
- return -1;
- /* XXX: this code should not depend on lock_user */
- if (!(p = lock_user(VERIFY_READ, addr, l, 1)))
- return -1;
+ p = lock_user(VERIFY_READ, addr, l, 1);
+ if (!p) {
+ goto out_close;
+ }
memcpy(buf, p, l);
unlock_user(p, addr, 0);
}
@@ -436,7 +456,12 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
buf += l;
addr += l;
}
- return 0;
+ ret = 0;
+out_close:
+ if (mem_fd != -1) {
+ close(mem_fd);
+ }
+ return ret;
}
#endif
--
2.43.0On 1/9/24 10:34, Ilya Leoshkevich wrote:
> gdbserver ignores page protection by virtue of using /proc/$pid/mem.
> Teach qemu gdbstub to do this too. This will not work if /proc is not
> mounted; accept this limitation.
>
> One alternative is to temporarily grant the missing PROT_* bit, but
> this is inherently racy. Another alternative is self-debugging with
> ptrace(POKE), which will break if QEMU itself is being debugged - a
> much more severe limitation.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> cpu-target.c | 55 ++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 40 insertions(+), 15 deletions(-)
>
> diff --git a/cpu-target.c b/cpu-target.c
> index 5eecd7ea2d7..69e97f78980 100644
> --- a/cpu-target.c
> +++ b/cpu-target.c
> @@ -406,6 +406,15 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
> vaddr l, page;
> void * p;
> uint8_t *buf = ptr;
> + int ret = -1;
> + int mem_fd;
> +
> + /*
> + * Try ptrace first. If /proc is not mounted or if there is a different
> + * problem, fall back to the manual page access. Note that, unlike ptrace,
> + * it will not be able to ignore the protection bits.
> + */
> + mem_fd = open("/proc/self/mem", is_write ? O_WRONLY : O_RDONLY);
Surely this is the unlikely fallback, and you don't need to open unless the page is
otherwise inaccessible.
I see no handling for writes to pages that contain TranslationBlocks.
r~
>
> while (len > 0) {
> page = addr & TARGET_PAGE_MASK;
> @@ -413,22 +422,33 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
> if (l > len)
> l = len;
> flags = page_get_flags(page);
> - if (!(flags & PAGE_VALID))
> - return -1;
> + if (!(flags & PAGE_VALID)) {
> + goto out_close;
> + }
> if (is_write) {
> - if (!(flags & PAGE_WRITE))
> - return -1;
> + if (mem_fd == -1 ||
> + pwrite(mem_fd, ptr, len, (off_t)g2h_untagged(addr)) != len) {
> + if (!(flags & PAGE_WRITE)) {
> + goto out_close;
> + }
> + /* XXX: this code should not depend on lock_user */
> + p = lock_user(VERIFY_WRITE, addr, l, 0);
> + if (!p) {
> + goto out_close;
> + }
> + memcpy(p, buf, l);
> + unlock_user(p, addr, l);
> + }
> + } else if (mem_fd == -1 ||
> + pread(mem_fd, ptr, len, (off_t)g2h_untagged(addr)) != len) {
> + if (!(flags & PAGE_READ)) {
> + goto out_close;
> + }
> /* XXX: this code should not depend on lock_user */
> - if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
> - return -1;
> - memcpy(p, buf, l);
> - unlock_user(p, addr, l);
> - } else {
> - if (!(flags & PAGE_READ))
> - return -1;
> - /* XXX: this code should not depend on lock_user */
> - if (!(p = lock_user(VERIFY_READ, addr, l, 1)))
> - return -1;
> + p = lock_user(VERIFY_READ, addr, l, 1);
> + if (!p) {
> + goto out_close;
> + }
> memcpy(buf, p, l);
> unlock_user(p, addr, 0);
> }
> @@ -436,7 +456,12 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
> buf += l;
> addr += l;
> }
> - return 0;
> + ret = 0;
> +out_close:
> + if (mem_fd != -1) {
> + close(mem_fd);
> + }
> + return ret;
> }
> #endif
>
On Wed, 2024-01-10 at 04:42 +1100, Richard Henderson wrote:
> On 1/9/24 10:34, Ilya Leoshkevich wrote:
> > gdbserver ignores page protection by virtue of using
> > /proc/$pid/mem.
> > Teach qemu gdbstub to do this too. This will not work if /proc is
> > not
> > mounted; accept this limitation.
> >
> > One alternative is to temporarily grant the missing PROT_* bit, but
> > this is inherently racy. Another alternative is self-debugging with
> > ptrace(POKE), which will break if QEMU itself is being debugged - a
> > much more severe limitation.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > cpu-target.c | 55 ++++++++++++++++++++++++++++++++++++++---------
> > -----
> > 1 file changed, 40 insertions(+), 15 deletions(-)
> >
> > diff --git a/cpu-target.c b/cpu-target.c
> > index 5eecd7ea2d7..69e97f78980 100644
> > --- a/cpu-target.c
> > +++ b/cpu-target.c
> > @@ -406,6 +406,15 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr
> > addr,
> > vaddr l, page;
> > void * p;
> > uint8_t *buf = ptr;
> > + int ret = -1;
> > + int mem_fd;
> > +
> > + /*
> > + * Try ptrace first. If /proc is not mounted or if there is a
> > different
> > + * problem, fall back to the manual page access. Note that,
> > unlike ptrace,
> > + * it will not be able to ignore the protection bits.
> > + */
> > + mem_fd = open("/proc/self/mem", is_write ? O_WRONLY :
> > O_RDONLY);
>
> Surely this is the unlikely fallback, and you don't need to open
> unless the page is
> otherwise inaccessible.
Ok, I can move this under (flags & PAGE_*) checks.
> I see no handling for writes to pages that contain TranslationBlocks.
Sorry, I completely missed that. I'm currently experimenting with the
following:
/*
* If there is a TranslationBlock and we weren't bypassing
host
* page protection, the memcpy() above would SEGV, ultimately
* leading to page_unprotect(). So invalidate the translations
* manually. Both invalidation and pwrite() must be under
* mmap_lock() in order to prevent the creation of another
* TranslationBlock in between.
*/
mmap_lock();
tb_invalidate_phys_page(page);
written = pwrite(fd, buf, l, (off_t)g2h_untagged(addr));
mmap_unlock();
Does that look okay?
[...]
On 1/10/24 06:39, Ilya Leoshkevich wrote:
> On Wed, 2024-01-10 at 04:42 +1100, Richard Henderson wrote:
>> On 1/9/24 10:34, Ilya Leoshkevich wrote:
>>> gdbserver ignores page protection by virtue of using
>>> /proc/$pid/mem.
>>> Teach qemu gdbstub to do this too. This will not work if /proc is
>>> not
>>> mounted; accept this limitation.
>>>
>>> One alternative is to temporarily grant the missing PROT_* bit, but
>>> this is inherently racy. Another alternative is self-debugging with
>>> ptrace(POKE), which will break if QEMU itself is being debugged - a
>>> much more severe limitation.
>>>
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>> cpu-target.c | 55 ++++++++++++++++++++++++++++++++++++++---------
>>> -----
>>> 1 file changed, 40 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/cpu-target.c b/cpu-target.c
>>> index 5eecd7ea2d7..69e97f78980 100644
>>> --- a/cpu-target.c
>>> +++ b/cpu-target.c
>>> @@ -406,6 +406,15 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr
>>> addr,
>>> vaddr l, page;
>>> void * p;
>>> uint8_t *buf = ptr;
>>> + int ret = -1;
>>> + int mem_fd;
>>> +
>>> + /*
>>> + * Try ptrace first. If /proc is not mounted or if there is a
>>> different
>>> + * problem, fall back to the manual page access. Note that,
>>> unlike ptrace,
>>> + * it will not be able to ignore the protection bits.
>>> + */
>>> + mem_fd = open("/proc/self/mem", is_write ? O_WRONLY :
>>> O_RDONLY);
>>
>> Surely this is the unlikely fallback, and you don't need to open
>> unless the page is
>> otherwise inaccessible.
>
> Ok, I can move this under (flags & PAGE_*) checks.
>
>> I see no handling for writes to pages that contain TranslationBlocks.
>
> Sorry, I completely missed that. I'm currently experimenting with the
> following:
>
> /*
> * If there is a TranslationBlock and we weren't bypassing
> host
> * page protection, the memcpy() above would SEGV, ultimately
> * leading to page_unprotect(). So invalidate the translations
> * manually. Both invalidation and pwrite() must be under
> * mmap_lock() in order to prevent the creation of another
> * TranslationBlock in between.
> */
> mmap_lock();
> tb_invalidate_phys_page(page);
> written = pwrite(fd, buf, l, (off_t)g2h_untagged(addr));
I would use here tb_invalidate_phys_range(addr, addr + l - 1),
but otherwise, it looks good.
r~
> mmap_unlock();
>
> Does that look okay?
>
> [...]
© 2016 - 2024 Red Hat, Inc.