[v3] tcg: Workaround macOS 11.2 mprotect bug

[PATCH v3 for-6.0 2/2] tcg: Workaround macOS 11.2 mprotect bug

Posted by Richard Henderson 4 years, 10 months ago

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.

As a short-term fix, ignore failures setting up the guard pages.

[1] https://gist.github.com/hikalium/75ae822466ee4da13cbbe486498a191f

Buglink: https://bugs.launchpad.net/qemu/+bug/1914849
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tcg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 88c9e6f8a4..1fbe0b686d 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -864,11 +864,15 @@ void tcg_region_init(void)
      */
     for (i = 0; i < region.n; i++) {
         void *start, *end;
-        int rc;
 
         tcg_region_bounds(i, &start, &end);
-        rc = qemu_mprotect_none(end, page_size);
-        g_assert(!rc);
+
+        /*
+         * 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.
+         */
+        (void)qemu_mprotect_none(end, page_size);
     }
 
     tcg_region_trees_init();
-- 
2.25.1

Re: [PATCH v3 for-6.0 2/2] tcg: Workaround macOS 11.2 mprotect bug

Posted by Philippe Mathieu-Daudé 4 years, 10 months ago

On 3/20/21 5:57 PM, Richard Henderson wrote:
> 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.
> 
> As a short-term fix, ignore failures setting up the guard pages.
> 
> [1] https://gist.github.com/hikalium/75ae822466ee4da13cbbe486498a191f
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1914849
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/tcg.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 88c9e6f8a4..1fbe0b686d 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -864,11 +864,15 @@ void tcg_region_init(void)
>       */
>      for (i = 0; i < region.n; i++) {
>          void *start, *end;
> -        int rc;
>  
>          tcg_region_bounds(i, &start, &end);
> -        rc = qemu_mprotect_none(end, page_size);

What about:

#ifdef CONFIG_DARWIN

           /* ... */
           (void)rc;
#else

> -        g_assert(!rc);

#endif

> +
> +        /*
> +         * 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.
> +         */
> +        (void)qemu_mprotect_none(end, page_size);
>      }
>  
>      tcg_region_trees_init();
>

Re: [PATCH v3 for-6.0 2/2] tcg: Workaround macOS 11.2 mprotect bug

Posted by Roman Bolshakov 4 years, 10 months ago

On Mon, Mar 22, 2021 at 11:03:05AM +0100, Philippe Mathieu-Daudé wrote:
> On 3/20/21 5:57 PM, Richard Henderson wrote:
> > 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.
> > 
> > As a short-term fix, ignore failures setting up the guard pages.
> > 
> > [1] https://gist.github.com/hikalium/75ae822466ee4da13cbbe486498a191f
> > 
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1914849
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  tcg/tcg.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tcg/tcg.c b/tcg/tcg.c
> > index 88c9e6f8a4..1fbe0b686d 100644
> > --- a/tcg/tcg.c
> > +++ b/tcg/tcg.c
> > @@ -864,11 +864,15 @@ void tcg_region_init(void)
> >       */
> >      for (i = 0; i < region.n; i++) {
> >          void *start, *end;
> > -        int rc;
> >  
> >          tcg_region_bounds(i, &start, &end);
> > -        rc = qemu_mprotect_none(end, page_size);
> 
> What about:
> 
> #ifdef CONFIG_DARWIN
> 
>            /* ... */
>            (void)rc;
> #else
> 
> > -        g_assert(!rc);
> 
> #endif
> 
> > +
> > +        /*
> > +         * 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.
> > +         */
> > +        (void)qemu_mprotect_none(end, page_size);
> >      }
> >  
> >      tcg_region_trees_init();
> > 
> 

I agree with Philippe, it's worth to keep the bug detection on non-buggy
platforms.

Otherwise:

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>

Thanks,
Roman

Re: [PATCH v3 for-6.0 2/2] tcg: Workaround macOS 11.2 mprotect bug

Posted by Richard Henderson 4 years, 10 months ago

On 3/22/21 4:03 AM, Philippe Mathieu-Daudé wrote:
>> -        rc = qemu_mprotect_none(end, page_size);
> 
> What about:
> 
> #ifdef CONFIG_DARWIN
> 
>             /* ... */
>             (void)rc;
> #else
> 
>> -        g_assert(!rc);
> 
> #endif

What does that buy us, really?  It seems like it just clutters the code with 
ifdefs.


r~

Re: [PATCH v3 for-6.0 2/2] tcg: Workaround macOS 11.2 mprotect bug

Posted by Philippe Mathieu-Daudé 4 years, 10 months ago

On 3/22/21 4:00 PM, Richard Henderson wrote:
> On 3/22/21 4:03 AM, Philippe Mathieu-Daudé wrote:
>>> -        rc = qemu_mprotect_none(end, page_size);
>>
>> What about:
>>
>> #ifdef CONFIG_DARWIN
>>
>>             /* ... */
>>             (void)rc;
>> #else
>>
>>> -        g_assert(!rc);
>>
>> #endif
> 
> What does that buy us, really?  It seems like it just clutters the code
> with ifdefs.

No problem.