| Message ID | Y5iwTaydU7i66K/i@p100 |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] linux-user: Add emulation for MADV_WIPEONFORK and MADV_KEEPONFORK in madvise() | expand |
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>
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>
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
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();
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