All of lore.kernel.org
 help / color / mirror / Atom feed
From: Helge Deller <deller@gmx.de>
To: Laurent Vivier <laurent@vivier.eu>,
	Ilya Leoshkevich <iii@linux.ibm.com>,
	Vitaly Buka <vitalybuka@google.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org
Subject: [PATCH v2] linux-user: Add emulation for MADV_WIPEONFORK and MADV_KEEPONFORK in madvise()
Date: Tue, 13 Dec 2022 18:03:09 +0100	[thread overview]
Message-ID: <Y5iwTaydU7i66K/i@p100> (raw)

Both parameters have a different value on the parisc platform, so first
translate the target value into a host value for usage in the native
madvise() syscall.

Those parameters are often used by security sensitive applications (e.g.
tor browser, boringssl, ...) which expect the call to return a proper
return code on failure, so return -EINVAL if qemu fails to forward the
syscall to the host OS.

While touching this code, enhance the comments about MADV_DONTNEED.

Tested with testcase of tor browser when running hppa-linux guest on
x86-64 host.

Signed-off-by: Helge Deller <deller@gmx.de>

---
v2: based on feedback from Ilya Leoshkevich <iii@linux.ibm.com>
- rename can_passthrough_madv_dontneed() to can_passthrough_madvise()
- rephrase the comment about MADV_DONTNEED

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 10f5079331..28135c9e6a 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -857,7 +857,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
     return new_addr;
 }

-static bool can_passthrough_madv_dontneed(abi_ulong start, abi_ulong end)
+static bool can_passthrough_madvise(abi_ulong start, abi_ulong end)
 {
     ulong addr;

@@ -901,23 +901,53 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, int advice)
         return -TARGET_EINVAL;
     }

+    /* Translate for some architectures which have different MADV_xxx values */
+    switch (advice) {
+    case TARGET_MADV_DONTNEED:      /* alpha */
+        advice = MADV_DONTNEED;
+        break;
+    case TARGET_MADV_WIPEONFORK:    /* parisc */
+        advice = MADV_WIPEONFORK;
+        break;
+    case TARGET_MADV_KEEPONFORK:    /* parisc */
+        advice = MADV_KEEPONFORK;
+        break;
+    /* we do not care about the other MADV_xxx values yet */
+    }
+
     /*
-     * A straight passthrough may not be safe because qemu sometimes turns
-     * private file-backed mappings into anonymous mappings.
+     * Most advice values are hints, so ignoring and returning success is ok.
+     *
+     * However, some advice values such as MADV_DONTNEED, MADV_WIPEONFORK and
+     * MADV_KEEPONFORK are not hints and need to be emulated.
      *
-     * This is a hint, so ignoring and returning success is ok.
+     * A straight passthrough for those may not be safe because qemu sometimes
+     * turns private file-backed mappings into anonymous mappings.
+     * can_passthrough_madvise() helps to check if a passthrough is possible by
+     * comparing mappings that are known to have the same semantics in the host
+     * and the guest. In this case passthrough is safe.
      *
-     * This breaks MADV_DONTNEED, completely implementing which is quite
-     * complicated. However, there is one low-hanging fruit: mappings that are
-     * known to have the same semantics in the host and the guest. In this case
-     * passthrough is safe, so do it.
+     * We pass through MADV_WIPEONFORK and MADV_KEEPONFORK if possible and
+     * return failure if not.
+     *
+     * MADV_DONTNEED is passed through as well, if possible.
+     * If passthrough isn't possible, we nevertheless (wrongly!) return
+     * success, which is broken but some userspace programs fail to work
+     * otherwise. Completely implementing such emulation is quite complicated
+     * though.
      */
     mmap_lock();
-    if (advice == TARGET_MADV_DONTNEED &&
-        can_passthrough_madv_dontneed(start, end)) {
-        ret = get_errno(madvise(g2h_untagged(start), len, MADV_DONTNEED));
-        if (ret == 0) {
-            page_reset_target_data(start, start + len);
+    switch (advice) {
+    case MADV_WIPEONFORK:
+    case MADV_KEEPONFORK:
+        ret = -EINVAL;
+        /* fall through */
+    case MADV_DONTNEED:
+        if (can_passthrough_madvise(start, end)) {
+            ret = get_errno(madvise(g2h_untagged(start), len, advice));
+            if ((advice == MADV_DONTNEED) && (ret == 0)) {
+                page_reset_target_data(start, start + len);
+            }
         }
     }
     mmap_unlock();


             reply	other threads:[~2022-12-13 17:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-13 17:03 Helge Deller [this message]
2022-12-13 17:19 ` [PATCH v2] linux-user: Add emulation for MADV_WIPEONFORK and MADV_KEEPONFORK in madvise() Ilya Leoshkevich
2023-01-30 12:52 ` Laurent Vivier
2023-01-30 12:53 ` Laurent Vivier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y5iwTaydU7i66K/i@p100 \
    --to=deller@gmx.de \
    --cc=iii@linux.ibm.com \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=vitalybuka@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.