If multiple guest threads in user-mode emulation write to a page which QEMU has marked read-only because of cached TCG translations, the threads can race in page_unprotect: * threads A & B both try to do a write to a page with code in it at the same time (ie which we've made non-writeable, so SEGV) * they race into the signal handler with this faulting address * thread A happens to get to page_unprotect() first and takes the mmap lock, so thread B sits waiting for it to be done * A then finds the page, marks it PAGE_WRITE and mprotect()s it writable * A can then continue OK (returns from signal handler to retry the memory access) * ...but when B gets the mmap lock it finds that the page is already PAGE_WRITE, and so it exits page_unprotect() via the "not due to protected translation" code path, and wrongly delivers the signal to the guest rather than just retrying the access In particular, this meant that trying to run 'javac' in user-mode emulation would fail with a spurious guest SIGSEGV. Handle this by making page_unprotect() assume that a call for a page which is already PAGE_WRITE is due to a race of this sort and return a "fault handled" indication. Since this would cause an infinite loop if we ever called page_unprotect() for some other kind of fault than "write failed due to bad access permissions", tighten the condition in handle_cpu_signal() to check the signal number and si_code, and add a comment so that if somebody does ever find themselves debugging an infinite loop of faults they have some clue about why. (The trick for identifying the correct setting for current_tb_invalidated for thread B (needed to handle the precise-SMC case) is due to Richard Henderson. Paolo Bonzini suggested just relying on si_code rather than trying anything more complicated.) I'm slightly nervous about assuming that we get SEGV with SEGV_ACCERR on a write if and only if the page permissions are set to non-writeable (for instance https://www.mail-archive.com/tech@openbsd.org/msg40358.html is a surprisingly recent patch to fix this for OpenBSD/x86, and I would be unsurprised if QEMU usermode itself got the si_code wrong, and the Boehm gc code at https://github.com/ivmai/bdwgc/blob/master/os_dep.c#L3182 is quite convoluted), but on the other hand we really ought in theory to be able to rely on si_code, and trying to handle this some other way gets pretty convoluted... thanks -- PMM Peter Maydell (2): linux-user: Propagate siginfo_t through to handle_cpu_signal() page_unprotect(): handle calls to pages that are PAGE_WRITE accel/tcg/translate-all.c | 50 ++++++++++++++++++++++++++++----------------- accel/tcg/user-exec.c | 52 +++++++++++++++++++++++------------------------ 2 files changed, 57 insertions(+), 45 deletions(-) -- 2.7.4
On 28 November 2017 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote: > If multiple guest threads in user-mode emulation write to a > page which QEMU has marked read-only because of cached TCG > translations, the threads can race in page_unprotect: > Peter Maydell (2): > linux-user: Propagate siginfo_t through to handle_cpu_signal() > page_unprotect(): handle calls to pages that are PAGE_WRITE Ping! Linux-user maintainers, any chance this could get into a pull-request sometime soon? (I have another cleanup I'm thinking of that will touch the same code so I'd rather this went into master before I look at that.) (I have a bunch of other pending linux-user patchsets which I shan't bother to ping individually unless you want me to.) thanks -- PMM
Le 15/01/2018 à 13:48, Peter Maydell a écrit : > On 28 November 2017 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote: >> If multiple guest threads in user-mode emulation write to a >> page which QEMU has marked read-only because of cached TCG >> translations, the threads can race in page_unprotect: > >> Peter Maydell (2): >> linux-user: Propagate siginfo_t through to handle_cpu_signal() >> page_unprotect(): handle calls to pages that are PAGE_WRITE > > Ping! Linux-user maintainers, any chance this could get into > a pull-request sometime soon? (I have another cleanup I'm > thinking of that will touch the same code so I'd rather this > went into master before I look at that.) > > (I have a bunch of other pending linux-user patchsets which > I shan't bother to ping individually unless you want me to.) Riku, if you have no time to prepare a pull request, I can. Thanks, Laurent
Hi Laurent, On Mon, Jan 15, 2018 at 01:52:32PM +0100, Laurent Vivier wrote: > Le 15/01/2018 à 13:48, Peter Maydell a écrit : > > On 28 November 2017 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote: > >> If multiple guest threads in user-mode emulation write to a > >> page which QEMU has marked read-only because of cached TCG > >> translations, the threads can race in page_unprotect: > > > >> Peter Maydell (2): > >> linux-user: Propagate siginfo_t through to handle_cpu_signal() > >> page_unprotect(): handle calls to pages that are PAGE_WRITE > > > > Ping! Linux-user maintainers, any chance this could get into > > a pull-request sometime soon? (I have another cleanup I'm > > thinking of that will touch the same code so I'd rather this > > went into master before I look at that.) > > > > (I have a bunch of other pending linux-user patchsets which > > I shan't bother to ping individually unless you want me to.) > > Riku, > > if you have no time to prepare a pull request, I can. Thanks, that would be great. Riku
Le 19/01/2018 à 15:51, Riku Voipio a écrit : > Hi Laurent, > > On Mon, Jan 15, 2018 at 01:52:32PM +0100, Laurent Vivier wrote: >> Le 15/01/2018 à 13:48, Peter Maydell a écrit : >>> On 28 November 2017 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> If multiple guest threads in user-mode emulation write to a >>>> page which QEMU has marked read-only because of cached TCG >>>> translations, the threads can race in page_unprotect: >>> >>>> Peter Maydell (2): >>>> linux-user: Propagate siginfo_t through to handle_cpu_signal() >>>> page_unprotect(): handle calls to pages that are PAGE_WRITE >>> >>> Ping! Linux-user maintainers, any chance this could get into >>> a pull-request sometime soon? (I have another cleanup I'm >>> thinking of that will touch the same code so I'd rather this >>> went into master before I look at that.) >>> >>> (I have a bunch of other pending linux-user patchsets which >>> I shan't bother to ping individually unless you want me to.) >> >> Riku, >> >> if you have no time to prepare a pull request, I can. > > Thanks, that would be great. OK, I'm going to prepare it. Thanks, Laurent
Le 19/01/2018 à 15:51, Riku Voipio a écrit : > Hi Laurent, > > On Mon, Jan 15, 2018 at 01:52:32PM +0100, Laurent Vivier wrote: >> Le 15/01/2018 à 13:48, Peter Maydell a écrit : >>> On 28 November 2017 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> If multiple guest threads in user-mode emulation write to a >>>> page which QEMU has marked read-only because of cached TCG >>>> translations, the threads can race in page_unprotect: >>> >>>> Peter Maydell (2): >>>> linux-user: Propagate siginfo_t through to handle_cpu_signal() >>>> page_unprotect(): handle calls to pages that are PAGE_WRITE >>> >>> Ping! Linux-user maintainers, any chance this could get into >>> a pull-request sometime soon? (I have another cleanup I'm >>> thinking of that will touch the same code so I'd rather this >>> went into master before I look at that.) >>> >>> (I have a bunch of other pending linux-user patchsets which >>> I shan't bother to ping individually unless you want me to.) >> >> Riku, >> >> if you have no time to prepare a pull request, I can. > > Thanks, that would be great. Riku, if you agree, I can continue to collect patches for the next pull-request. It's easier to collect patches on the fly than scanning the ML when we want to create a pull request. Thanks, Laurent
Le 15/01/2018 à 13:48, Peter Maydell a écrit : > On 28 November 2017 at 14:35, Peter Maydell <peter.maydell@linaro.org> wrote: >> If multiple guest threads in user-mode emulation write to a >> page which QEMU has marked read-only because of cached TCG >> translations, the threads can race in page_unprotect: > >> Peter Maydell (2): >> linux-user: Propagate siginfo_t through to handle_cpu_signal() >> page_unprotect(): handle calls to pages that are PAGE_WRITE > > Ping! Linux-user maintainers, any chance this could get into > a pull-request sometime soon? (I have another cleanup I'm > thinking of that will touch the same code so I'd rather this > went into master before I look at that.) > > (I have a bunch of other pending linux-user patchsets which > I shan't bother to ping individually unless you want me to.) > > thanks > -- PMM > I've applied this series to my linux-user branch. I've tried to find all pending linux-user patches. If you want to check none of yours is is missing, all patches I have collected are in: https://github.com/vivier/qemu/commits/linux-user-for-2.12 I'm going to run some tests before sending the pull request. Thanks, Laurent
© 2016 - 2024 Red Hat, Inc.