QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH v3 27/28] tcg: When allocating for !splitwx, begin with PROT_NONE
Date: Wed, 09 Jun 2021 12:21:16 +0100	[thread overview]
Message-ID: <875yynjn72.fsf@linaro.org> (raw)
In-Reply-To: <20210502231844.1977630-28-richard.henderson@linaro.org>


Richard Henderson <richard.henderson@linaro.org> writes:

> There's a change in mprotect() behaviour [1] in the latest macOS
> on M1 and it's not yet clear if it's going to be fixed by Apple.
>
> In this case, instead of changing permissions of N guard pages,
> we change permissions of N rwx regions.  The same number of
> syscalls are required either way.
>
> [1] https://gist.github.com/hikalium/75ae822466ee4da13cbbe486498a191f
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/region.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/tcg/region.c b/tcg/region.c
> index 604530b902..5e00db4cfb 100644
> --- a/tcg/region.c
> +++ b/tcg/region.c
> @@ -765,12 +765,15 @@ static int alloc_code_gen_buffer(size_t size, int splitwx, Error **errp)
>          error_free_or_abort(errp);
>      }
>  
> -    prot = PROT_READ | PROT_WRITE | PROT_EXEC;
> +    /*
> +     * macOS 11.2 has a bug (Apple Feedback FB8994773) in which mprotect
> +     * rejects a permission change from RWX -> NONE when reserving the
> +     * guard pages later.  We can go the other way with the same number
> +     * of syscalls, so always begin with PROT_NONE.
> +     */
> +    prot = PROT_NONE;
>      flags = MAP_PRIVATE | MAP_ANONYMOUS;
> -#ifdef CONFIG_TCG_INTERPRETER
> -    /* The tcg interpreter does not need execute permission. */
> -    prot = PROT_READ | PROT_WRITE;
> -#elif defined(CONFIG_DARWIN)
> +#ifdef CONFIG_DARWIN
>      /* Applicable to both iOS and macOS (Apple Silicon). */
>      if (!splitwx) {
>          flags |= MAP_JIT;
> @@ -901,11 +904,7 @@ void tcg_region_init(size_t tb_size, int splitwx, unsigned max_cpus)
>              }
>          }

I think the comment in tcg_region_init needs to be updated, currently
reads:

    /*
     * Set guard pages in the rw buffer, as that's the one into which
     * buffer overruns could occur.  Do not set guard pages in the rx
     * buffer -- let that one use hugepages throughout.
     * Work with the page protections set up with the initial mapping.
     */
    need_prot = PAGE_READ | PAGE_WRITE;

but now we start with everything at PROT_NONE and are just setting
need_prot for the non-guard pages.

>          if (have_prot != 0) {
> -            /*
> -             * macOS 11.2 has a bug (Apple Feedback FB8994773) in which mprotect
> -             * rejects a permission change from RWX -> NONE.  Guard pages are
> -             * nice for bug detection but are not essential; ignore any failure.
> -             */
> +            /* Guard pages are nice for bug detection but are not essential. */
>              (void)qemu_mprotect_none(end, page_size);

Isn't the last page already set as PROT_NONE?

>          }
>      }


-- 
Alex Bennée


  reply	other threads:[~2021-06-09 11:24 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-02 23:18 [PATCH v3 00/28] tcg: Clean up code_gen_buffer allocation Richard Henderson
2021-05-02 23:18 ` [PATCH v3 01/28] meson: Split out tcg/meson.build Richard Henderson
2021-06-07 19:38   ` Alex Bennée
2021-06-09 14:56   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 02/28] meson: Split out fpu/meson.build Richard Henderson
2021-06-07 19:38   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 03/28] tcg: Re-order tcg_region_init vs tcg_prologue_init Richard Henderson
2021-06-08  9:04   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 04/28] tcg: Remove error return from tcg_region_initial_alloc__locked Richard Henderson
2021-06-08 11:04   ` Alex Bennée
2021-06-08 15:44     ` Richard Henderson
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 05/28] tcg: Split out tcg_region_initial_alloc Richard Henderson
2021-06-08 11:45   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 06/28] tcg: Split out tcg_region_prologue_set Richard Henderson
2021-06-08 11:47   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 07/28] tcg: Split out region.c Richard Henderson
2021-06-08 11:48   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 08/28] accel/tcg: Inline cpu_gen_init Richard Henderson
2021-06-08 11:49   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 09/28] accel/tcg: Move alloc_code_gen_buffer to tcg/region.c Richard Henderson
2021-06-08 11:50   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 10/28] accel/tcg: Rename tcg_init to tcg_init_machine Richard Henderson
2021-06-08 11:51   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 11/28] tcg: Create tcg_init Richard Henderson
2021-06-08 11:52   ` Alex Bennée
2021-06-09 14:57   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 12/28] accel/tcg: Merge tcg_exec_init into tcg_init_machine Richard Henderson
2021-06-08 11:55   ` Alex Bennée
2021-06-08 15:45     ` Richard Henderson
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-06-10 15:16     ` Richard Henderson
2021-05-02 23:18 ` [PATCH v3 13/28] accel/tcg: Pass down max_cpus to tcg_init Richard Henderson
2021-06-08 15:44   ` Alex Bennée
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 14/28] tcg: Introduce tcg_max_ctxs Richard Henderson
2021-06-08 15:54   ` Alex Bennée
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 15/28] tcg: Move MAX_CODE_GEN_BUFFER_SIZE to tcg-target.h Richard Henderson
2021-06-08 15:56   ` Alex Bennée
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 16/28] tcg: Replace region.end with region.total_size Richard Henderson
2021-06-08 16:03   ` Alex Bennée
2021-06-08 16:12     ` Richard Henderson
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 17/28] tcg: Rename region.start to region.after_prologue Richard Henderson
2021-06-08 16:06   ` Alex Bennée
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 18/28] tcg: Tidy tcg_n_regions Richard Henderson
2021-06-08 16:06   ` Alex Bennée
2021-06-09 14:58   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 19/28] tcg: Tidy split_cross_256mb Richard Henderson
2021-06-09 10:01   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-06-10 15:20     ` Richard Henderson
2021-05-02 23:18 ` [PATCH v3 20/28] tcg: Move in_code_gen_buffer and tests to region.c Richard Henderson
2021-06-09 10:07   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 21/28] tcg: Allocate code_gen_buffer into struct tcg_region_state Richard Henderson
2021-06-09 10:29   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 22/28] tcg: Return the map protection from alloc_code_gen_buffer Richard Henderson
2021-06-09 10:30   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 23/28] tcg: Sink qemu_madvise call to common code Richard Henderson
2021-06-09 10:34   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 24/28] util/osdep: Add qemu_mprotect_rw Richard Henderson
2021-06-09 10:34   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 25/28] tcg: Round the tb_size default from qemu_get_host_physmem Richard Henderson
2021-06-09 10:36   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 26/28] tcg: Merge buffer protection and guard page protection Richard Henderson
2021-06-09 10:39   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 27/28] tcg: When allocating for !splitwx, begin with PROT_NONE Richard Henderson
2021-06-09 11:21   ` Alex Bennée [this message]
2021-06-10 15:34     ` Richard Henderson
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:18 ` [PATCH v3 28/28] tcg: Move tcg_init_ctx and tcg_ctx from accel/tcg/ Richard Henderson
2021-06-09 11:30   ` Alex Bennée
2021-06-09 14:59   ` Luis Fernando Fujita Pires
2021-05-02 23:45 ` [PATCH v3 00/28] tcg: Clean up code_gen_buffer allocation no-reply

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=875yynjn72.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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 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 read-only IMAP folder(s) and NNTP newsgroup(s).