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();
Le 13/12/2022 à 18:03, Helge Deller a écrit : > 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(); > Applied to my linux-user-for-8.0 branch. Thanks, Laurent
Le 13/12/2022 à 18:03, Helge Deller a écrit : > 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(); > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On Tue, Dec 13, 2022 at 06:03:09PM +0100, Helge Deller wrote: > 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 Thanks! Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
© 2016 - 2024 Red Hat, Inc.