target/i386: fix missing PF_INSTR in SIGSEGV context

[PATCH] target/i386: fix missing PF_INSTR in SIGSEGV context

Posted by Simon Scherer 1 month, 2 weeks ago

When running linux-user emulation, the SIGSEGV handler does not
correctly set the 4th bit (PF_INSTR) in the error_code variable of
the context argument (context->uc_mcontext.gregs[REG_ERR]).

Because this bit is never set, guest applications cannot distinguish
if a fault was due to missing executable permissions. This patch
ensures that when a page fault occurs during an instruction fetch,
the PF_INSTR flag is properly populated in the signal context.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3384
Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
---
 VERSION                            | 2 +-
 target/i386/tcg/user/excp_helper.c | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/VERSION b/VERSION
index de760e732f..340ebebb29 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-10.2.92
+10.2.93
diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c
index 98fab4cbc3..6c5df5e0e8 100644
--- a/target/i386/tcg/user/excp_helper.c
+++ b/target/i386/tcg/user/excp_helper.c
@@ -36,9 +36,10 @@ void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr,
      * signal and set exception_index to EXCP_INTERRUPT.
      */
     env->cr[2] = addr;
-    env->error_code = ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)
-                    | (maperr ? 0 : PG_ERROR_P_MASK)
-                    | PG_ERROR_U_MASK;
+    env->error_code = (maperr ? 0 : PG_ERROR_P_MASK)
+                    | ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)
+                    | PG_ERROR_U_MASK
+                    | ((access_type == MMU_INST_FETCH) ? PG_ERROR_I_D_MASK : 0);
     cs->exception_index = EXCP0E_PAGE;
 
     /* Disable do_interrupt_user. */
-- 
2.53.0

Re: [PATCH] target/i386: fix missing PF_INSTR in SIGSEGV context

Posted by Paolo Bonzini 1 month, 2 weeks ago

Queued, thanks.

Paolo

Re: [PATCH] target/i386: fix missing PF_INSTR in SIGSEGV context

Posted by Peter Maydell 1 month, 2 weeks ago

On Mon, 13 Apr 2026 at 13:17, Simon Scherer <scherer.simon89@gmail.com> wrote:
>
> When running linux-user emulation, the SIGSEGV handler does not
> correctly set the 4th bit (PF_INSTR) in the error_code variable of
> the context argument (context->uc_mcontext.gregs[REG_ERR]).
>
> Because this bit is never set, guest applications cannot distinguish
> if a fault was due to missing executable permissions. This patch
> ensures that when a page fault occurs during an instruction fetch,
> the PF_INSTR flag is properly populated in the signal context.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3384
> Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
> ---
>  VERSION                            | 2 +-
>  target/i386/tcg/user/excp_helper.c | 7 ++++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/VERSION b/VERSION
> index de760e732f..340ebebb29 100644
> --- a/VERSION
> +++ b/VERSION
> @@ -1 +1 @@
> -10.2.92
> +10.2.93

Your patch seems to have a stray other change in it.

You don't need to respin it just to fix this, and if it gets
reviewed without needing any other changes then the maintainer
will probably fix that up, but if you need to do a v2 of the
patch then you can sort it out for that.

> diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c
> index 98fab4cbc3..6c5df5e0e8 100644
> --- a/target/i386/tcg/user/excp_helper.c
> +++ b/target/i386/tcg/user/excp_helper.c
> @@ -36,9 +36,10 @@ void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr,
>       * signal and set exception_index to EXCP_INTERRUPT.
>       */
>      env->cr[2] = addr;
> -    env->error_code = ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)
> -                    | (maperr ? 0 : PG_ERROR_P_MASK)
> -                    | PG_ERROR_U_MASK;
> +    env->error_code = (maperr ? 0 : PG_ERROR_P_MASK)
> +                    | ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)

Why did you reorder these two lines ? Is there a code change here I'm missing?

> +                    | PG_ERROR_U_MASK
> +                    | ((access_type == MMU_INST_FETCH) ? PG_ERROR_I_D_MASK : 0);
>      cs->exception_index = EXCP0E_PAGE;
>
>      /* Disable do_interrupt_user. */

thanks
-- PMM

Re: [PATCH] target/i386: fix missing PF_INSTR in SIGSEGV context

Posted by Paolo Bonzini 1 month, 2 weeks ago

On 4/13/26 14:35, Peter Maydell wrote:
>> diff --git a/VERSION b/VERSION
>> index de760e732f..340ebebb29 100644
>> --- a/VERSION
>> +++ b/VERSION
>> @@ -1 +1 @@
>> -10.2.92
>> +10.2.93
> 
> Your patch seems to have a stray other change in it.
> 
> You don't need to respin it just to fix this, and if it gets
> reviewed without needing any other changes then the maintainer
> will probably fix that up, but if you need to do a v2 of the
> patch then you can sort it out for that.

Will do.

>> diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c
>> index 98fab4cbc3..6c5df5e0e8 100644
>> --- a/target/i386/tcg/user/excp_helper.c
>> +++ b/target/i386/tcg/user/excp_helper.c
>> @@ -36,9 +36,10 @@ void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr,
>>        * signal and set exception_index to EXCP_INTERRUPT.
>>        */
>>       env->cr[2] = addr;
>> -    env->error_code = ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)
>> -                    | (maperr ? 0 : PG_ERROR_P_MASK)
>> -                    | PG_ERROR_U_MASK;
>> +    env->error_code = (maperr ? 0 : PG_ERROR_P_MASK)
>> +                    | ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)
> 
> Why did you reorder these two lines ? Is there a code change here I'm missing?

It sorts bits in the order they appear in the word, so it makes sense.

Paolo

>> +                    | PG_ERROR_U_MASK
>> +                    | ((access_type == MMU_INST_FETCH) ? PG_ERROR_I_D_MASK : 0);
>>       cs->exception_index = EXCP0E_PAGE;
>>
>>       /* Disable do_interrupt_user. */
> 
> thanks
> -- PMM
> 
>