qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest
@ 2014-07-28 12:02 Mikhail Ilin
  2014-08-05  5:27 ` Mikhail Ilin
  0 siblings, 1 reply; 3+ messages in thread
From: Mikhail Ilin @ 2014-07-28 12:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: mst, hutao, riku.voipio, anthony, pbonzini, afaerber

Hi,

As it was posted earlier the output of reading /proc/self/maps is not
correct for a guest. There are some issues:

https://bugs.launchpad.net/qemu/+bug/1346784
http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03085.html
http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02793.html

The patch proposes: build /proc/self/maps doing a match against guest memory
translation table and output only that map records which are valid for guest
memory layout.

Patches in mentioned threads are not relevant and are covered by the current
patch.

We did some local tests for i386, x86_64 and arm targets. The approach
seems correct.


 From 8479d3dd00194975d7016eeecba13ddf453e9647 Mon Sep 17 00:00:00 2001
From: Mikhail Ilyin <m.ilin@samsung.com>
Date: Mon, 28 Jul 2014 15:40:31 +0400
Subject: [PATCH] Build /proc/self/maps doing a match against guest memory
  translation table. Output only that map records which are valid for guest
  memory layout.

Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
---
  include/exec/cpu-all.h |  2 ++
  linux-user/syscall.c   | 25 ++++++++++---------------
  2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f91581f..f9d132f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
  #define RESERVED_VA 0ul
  #endif

+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
+                                    (1ul << 
TARGET_VIRT_ADDR_SPACE_BITS) - 1)
  #endif

  /* page related stuff */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..189a8c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)

  static int open_self_maps(void *cpu_env, int fd)
  {
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || 
defined(TARGET_UNICORE32)
      CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
      TaskState *ts = cpu->opaque;
-#endif
      FILE *fp;
      char *line = NULL;
      size_t len = 0;
@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
          if ((fields < 10) || (fields > 11)) {
              continue;
          }
-        if (!strncmp(path, "[stack]", 7)) {
-            continue;
-        }
-        if (h2g_valid(min) && h2g_valid(max)) {
+        if (h2g_valid(min)) {
+            int flags = page_get_flags(h2g(min));
+            max = h2g_valid(max - 1) ? max : (uint64_t)g2h(GUEST_ADDR_MAX);
+            if (page_check_range(h2g(min), max - min, flags) == -1) {
+                continue;
+            }
+            if (h2g(min) == ts->info->stack_limit) {
+                pstrcpy(path, sizeof(path), "      [stack]");
+            }
              dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
                      " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
-                    h2g(min), h2g(max), flag_r, flag_w,
+                    h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
                      flag_x, flag_p, offset, dev_maj, dev_min, inode,
                      path[0] ? "         " : "", path);
          }
@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
      free(line);
      fclose(fp);

-#if defined(TARGET_ARM) || defined(TARGET_M68K) || 
defined(TARGET_UNICORE32)
-    dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0          [stack]\n",
-                (unsigned long long)ts->info->stack_limit,
-                (unsigned long long)(ts->info->start_stack +
-                                     (TARGET_PAGE_SIZE - 1)) & 
TARGET_PAGE_MASK,
-                (unsigned long long)0);
-#endif
-
      return 0;
  }

-- 
1.9.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest
  2014-07-28 12:02 [Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest Mikhail Ilin
@ 2014-08-05  5:27 ` Mikhail Ilin
  2014-08-05  7:06   ` Riku Voipio
  0 siblings, 1 reply; 3+ messages in thread
From: Mikhail Ilin @ 2014-08-05  5:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: mst, hutao, riku.voipio, anthony, pbonzini, afaerber

ping

http://patchwork.ozlabs.org/patch/374162/

On 28.07.2014 16:02, Mikhail Ilin wrote:
> Hi,
>
> As it was posted earlier the output of reading /proc/self/maps is not
> correct for a guest. There are some issues:
>
> https://bugs.launchpad.net/qemu/+bug/1346784
> http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03085.html
> http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02793.html
>
> The patch proposes: build /proc/self/maps doing a match against guest
> memory
> translation table and output only that map records which are valid for
> guest
> memory layout.
>
> Patches in mentioned threads are not relevant and are covered by the
> current
> patch.
>
> We did some local tests for i386, x86_64 and arm targets. The approach
> seems correct.
>
>
>  From 8479d3dd00194975d7016eeecba13ddf453e9647 Mon Sep 17 00:00:00 2001
> From: Mikhail Ilyin <m.ilin@samsung.com>
> Date: Mon, 28 Jul 2014 15:40:31 +0400
> Subject: [PATCH] Build /proc/self/maps doing a match against guest memory
>   translation table. Output only that map records which are valid for guest
>   memory layout.
>
> Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
> ---
>   include/exec/cpu-all.h |  2 ++
>   linux-user/syscall.c   | 25 ++++++++++---------------
>   2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f91581f..f9d132f 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -198,6 +198,8 @@ extern unsigned long reserved_va;
>   #define RESERVED_VA 0ul
>   #endif
>
> +#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
> +                                    (1ul <<
> TARGET_VIRT_ADDR_SPACE_BITS) - 1)
>   #endif
>
>   /* page related stuff */
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a50229d..189a8c0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)
>
>   static int open_self_maps(void *cpu_env, int fd)
>   {
> -#if defined(TARGET_ARM) || defined(TARGET_M68K) ||
> defined(TARGET_UNICORE32)
>       CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
>       TaskState *ts = cpu->opaque;
> -#endif
>       FILE *fp;
>       char *line = NULL;
>       size_t len = 0;
> @@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
>           if ((fields < 10) || (fields > 11)) {
>               continue;
>           }
> -        if (!strncmp(path, "[stack]", 7)) {
> -            continue;
> -        }
> -        if (h2g_valid(min) && h2g_valid(max)) {
> +        if (h2g_valid(min)) {
> +            int flags = page_get_flags(h2g(min));
> +            max = h2g_valid(max - 1) ? max :
> (uint64_t)g2h(GUEST_ADDR_MAX);
> +            if (page_check_range(h2g(min), max - min, flags) == -1) {
> +                continue;
> +            }
> +            if (h2g(min) == ts->info->stack_limit) {
> +                pstrcpy(path, sizeof(path), "      [stack]");
> +            }
>               dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
>                       " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
> -                    h2g(min), h2g(max), flag_r, flag_w,
> +                    h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
>                       flag_x, flag_p, offset, dev_maj, dev_min, inode,
>                       path[0] ? "         " : "", path);
>           }
> @@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
>       free(line);
>       fclose(fp);
>
> -#if defined(TARGET_ARM) || defined(TARGET_M68K) ||
> defined(TARGET_UNICORE32)
> -    dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0          [stack]\n",
> -                (unsigned long long)ts->info->stack_limit,
> -                (unsigned long long)(ts->info->start_stack +
> -                                     (TARGET_PAGE_SIZE - 1)) &
> TARGET_PAGE_MASK,
> -                (unsigned long long)0);
> -#endif
> -
>       return 0;
>   }
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest
  2014-08-05  5:27 ` Mikhail Ilin
@ 2014-08-05  7:06   ` Riku Voipio
  0 siblings, 0 replies; 3+ messages in thread
From: Riku Voipio @ 2014-08-05  7:06 UTC (permalink / raw)
  To: Mikhail Ilin; +Cc: mst, hutao, riku.voipio, qemu-devel, pbonzini, afaerber

Hi,

The patch in patchwork doesn't apply, even with a bit of editing. You
need to send the patch to list with git send-email instead of including
the patch inline. We have docs at:

http://wiki.qemu.org/Contribute/SubmitAPatch

Also, please include the description of the patch in the commit message,
rather than just in the mail. Thus it gets included in the git history and
people can find out why the patch have been applied more easily.

The patch itself looks correct, but indeed couldn't test it yet.

Riku

On Tue, Aug 05, 2014 at 09:27:05AM +0400, Mikhail Ilin wrote:
> ping
> 
> http://patchwork.ozlabs.org/patch/374162/
> 
> On 28.07.2014 16:02, Mikhail Ilin wrote:
> >Hi,
> >
> >As it was posted earlier the output of reading /proc/self/maps is not
> >correct for a guest. There are some issues:
> >
> >https://bugs.launchpad.net/qemu/+bug/1346784
> >http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03085.html
> >http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02793.html
> >
> >The patch proposes: build /proc/self/maps doing a match against guest
> >memory
> >translation table and output only that map records which are valid for
> >guest
> >memory layout.
> >
> >Patches in mentioned threads are not relevant and are covered by the
> >current
> >patch.
> >
> >We did some local tests for i386, x86_64 and arm targets. The approach
> >seems correct.
> >
> >
> > From 8479d3dd00194975d7016eeecba13ddf453e9647 Mon Sep 17 00:00:00 2001
> >From: Mikhail Ilyin <m.ilin@samsung.com>
> >Date: Mon, 28 Jul 2014 15:40:31 +0400
> >Subject: [PATCH] Build /proc/self/maps doing a match against guest memory
> >  translation table. Output only that map records which are valid for guest
> >  memory layout.
> >
> >Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
> >---
> >  include/exec/cpu-all.h |  2 ++
> >  linux-user/syscall.c   | 25 ++++++++++---------------
> >  2 files changed, 12 insertions(+), 15 deletions(-)
> >
> >diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> >index f91581f..f9d132f 100644
> >--- a/include/exec/cpu-all.h
> >+++ b/include/exec/cpu-all.h
> >@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
> >  #define RESERVED_VA 0ul
> >  #endif
> >
> >+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
> >+                                    (1ul <<
> >TARGET_VIRT_ADDR_SPACE_BITS) - 1)
> >  #endif
> >
> >  /* page related stuff */
> >diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> >index a50229d..189a8c0 100644
> >--- a/linux-user/syscall.c
> >+++ b/linux-user/syscall.c
> >@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)
> >
> >  static int open_self_maps(void *cpu_env, int fd)
> >  {
> >-#if defined(TARGET_ARM) || defined(TARGET_M68K) ||
> >defined(TARGET_UNICORE32)
> >      CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
> >      TaskState *ts = cpu->opaque;
> >-#endif
> >      FILE *fp;
> >      char *line = NULL;
> >      size_t len = 0;
> >@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
> >          if ((fields < 10) || (fields > 11)) {
> >              continue;
> >          }
> >-        if (!strncmp(path, "[stack]", 7)) {
> >-            continue;
> >-        }
> >-        if (h2g_valid(min) && h2g_valid(max)) {
> >+        if (h2g_valid(min)) {
> >+            int flags = page_get_flags(h2g(min));
> >+            max = h2g_valid(max - 1) ? max :
> >(uint64_t)g2h(GUEST_ADDR_MAX);
> >+            if (page_check_range(h2g(min), max - min, flags) == -1) {
> >+                continue;
> >+            }
> >+            if (h2g(min) == ts->info->stack_limit) {
> >+                pstrcpy(path, sizeof(path), "      [stack]");
> >+            }
> >              dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
> >                      " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
> >-                    h2g(min), h2g(max), flag_r, flag_w,
> >+                    h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
> >                      flag_x, flag_p, offset, dev_maj, dev_min, inode,
> >                      path[0] ? "         " : "", path);
> >          }
> >@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
> >      free(line);
> >      fclose(fp);
> >
> >-#if defined(TARGET_ARM) || defined(TARGET_M68K) ||
> >defined(TARGET_UNICORE32)
> >-    dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0          [stack]\n",
> >-                (unsigned long long)ts->info->stack_limit,
> >-                (unsigned long long)(ts->info->start_stack +
> >-                                     (TARGET_PAGE_SIZE - 1)) &
> >TARGET_PAGE_MASK,
> >-                (unsigned long long)0);
> >-#endif
> >-
> >      return 0;
> >  }
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-08-05  7:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-28 12:02 [Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest Mikhail Ilin
2014-08-05  5:27 ` Mikhail Ilin
2014-08-05  7:06   ` Riku Voipio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).