diff mbox series

[v2,10/23] linux-user/i386: Implement setup_sigtramp

Message ID 20210618192951.125651-11-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series linux-user: Move signal trampolines to new page | expand

Commit Message

Richard Henderson June 18, 2021, 7:29 p.m. UTC
Create and record the two signal trampolines.
Use them when the guest does not use SA_RESTORER.
Note that x86_64 does not use this code.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/i386/target_signal.h   |  2 ++
 linux-user/x86_64/target_signal.h |  3 +++
 linux-user/i386/signal.c          | 42 ++++++++++++++++++-------------
 3 files changed, 29 insertions(+), 18 deletions(-)

Comments

Peter Maydell June 29, 2021, 2:40 p.m. UTC | #1
On Fri, 18 Jun 2021 at 20:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create and record the two signal trampolines.
> Use them when the guest does not use SA_RESTORER.
> Note that x86_64 does not use this code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/i386/target_signal.h   |  2 ++
>  linux-user/x86_64/target_signal.h |  3 +++
>  linux-user/i386/signal.c          | 42 ++++++++++++++++++-------------
>  3 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h
> index 50361af874..64d09f2e75 100644
> --- a/linux-user/i386/target_signal.h
> +++ b/linux-user/i386/target_signal.h
> @@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
>  #include "../generic/signal.h"
>
>  #define TARGET_ARCH_HAS_SETUP_FRAME
> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
> +
>  #endif /* I386_TARGET_SIGNAL_H */
> diff --git a/linux-user/x86_64/target_signal.h b/linux-user/x86_64/target_signal.h
> index 4ea74f20dd..4673c5a886 100644
> --- a/linux-user/x86_64/target_signal.h
> +++ b/linux-user/x86_64/target_signal.h
> @@ -21,4 +21,7 @@ typedef struct target_sigaltstack {
>
>  #include "../generic/signal.h"
>
> +/* For x86_64, use of SA_RESTORER is mandatory. */
> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
> +
>  #endif /* X86_64_TARGET_SIGNAL_H */
> diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
> index 8701774e37..a83ecba54f 100644
> --- a/linux-user/i386/signal.c
> +++ b/linux-user/i386/signal.c
> @@ -337,16 +337,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
>      if (ka->sa_flags & TARGET_SA_RESTORER) {
>          __put_user(ka->sa_restorer, &frame->pretcode);
>      } else {
> -        uint16_t val16;
> -        abi_ulong retcode_addr;
> -        retcode_addr = frame_addr + offsetof(struct sigframe, retcode);
> -        __put_user(retcode_addr, &frame->pretcode);
> -        /* This is popl %eax ; movl $,%eax ; int $0x80 */
> -        val16 = 0xb858;
> -        __put_user(val16, (uint16_t *)(frame->retcode+0));
> -        __put_user(TARGET_NR_sigreturn, (int *)(frame->retcode+2));
> -        val16 = 0x80cd;
> -        __put_user(val16, (uint16_t *)(frame->retcode+6));
> +        __put_user(default_sigreturn, &frame->pretcode);
>

In the kernel in arch/x86/kernel/signal.c there is a comment:

        /*
         * This is popl %eax ; movl $__NR_sigreturn, %eax ; int $0x80
         *
         * WE DO NOT USE IT ANY MORE! It's only left here for historical
         * reasons and because gdb uses it as a signature to notice
         * signal handler stack frames.
         */

which suggests that we also should continue to fill in the
retcode bytes in the signal frame for gdb's benefit even though
we don't actually execute them any more.

thanks
-- PMM
Richard Henderson June 29, 2021, 6:30 p.m. UTC | #2
On 6/29/21 7:40 AM, Peter Maydell wrote:
> On Fri, 18 Jun 2021 at 20:38, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Create and record the two signal trampolines.
>> Use them when the guest does not use SA_RESTORER.
>> Note that x86_64 does not use this code.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/i386/target_signal.h   |  2 ++
>>   linux-user/x86_64/target_signal.h |  3 +++
>>   linux-user/i386/signal.c          | 42 ++++++++++++++++++-------------
>>   3 files changed, 29 insertions(+), 18 deletions(-)
>>
>> diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h
>> index 50361af874..64d09f2e75 100644
>> --- a/linux-user/i386/target_signal.h
>> +++ b/linux-user/i386/target_signal.h
>> @@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
>>   #include "../generic/signal.h"
>>
>>   #define TARGET_ARCH_HAS_SETUP_FRAME
>> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
>> +
>>   #endif /* I386_TARGET_SIGNAL_H */
>> diff --git a/linux-user/x86_64/target_signal.h b/linux-user/x86_64/target_signal.h
>> index 4ea74f20dd..4673c5a886 100644
>> --- a/linux-user/x86_64/target_signal.h
>> +++ b/linux-user/x86_64/target_signal.h
>> @@ -21,4 +21,7 @@ typedef struct target_sigaltstack {
>>
>>   #include "../generic/signal.h"
>>
>> +/* For x86_64, use of SA_RESTORER is mandatory. */
>> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
>> +
>>   #endif /* X86_64_TARGET_SIGNAL_H */
>> diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
>> index 8701774e37..a83ecba54f 100644
>> --- a/linux-user/i386/signal.c
>> +++ b/linux-user/i386/signal.c
>> @@ -337,16 +337,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
>>       if (ka->sa_flags & TARGET_SA_RESTORER) {
>>           __put_user(ka->sa_restorer, &frame->pretcode);
>>       } else {
>> -        uint16_t val16;
>> -        abi_ulong retcode_addr;
>> -        retcode_addr = frame_addr + offsetof(struct sigframe, retcode);
>> -        __put_user(retcode_addr, &frame->pretcode);
>> -        /* This is popl %eax ; movl $,%eax ; int $0x80 */
>> -        val16 = 0xb858;
>> -        __put_user(val16, (uint16_t *)(frame->retcode+0));
>> -        __put_user(TARGET_NR_sigreturn, (int *)(frame->retcode+2));
>> -        val16 = 0x80cd;
>> -        __put_user(val16, (uint16_t *)(frame->retcode+6));
>> +        __put_user(default_sigreturn, &frame->pretcode);
>>
> 
> In the kernel in arch/x86/kernel/signal.c there is a comment:
> 
>          /*
>           * This is popl %eax ; movl $__NR_sigreturn, %eax ; int $0x80
>           *
>           * WE DO NOT USE IT ANY MORE! It's only left here for historical
>           * reasons and because gdb uses it as a signature to notice
>           * signal handler stack frames.
>           */
> 
> which suggests that we also should continue to fill in the
> retcode bytes in the signal frame for gdb's benefit even though
> we don't actually execute them any more.

Point.  I noticed the same for several other targets, but didn't actually look at the 
kernel code for i386.


r~
diff mbox series

Patch

diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h
index 50361af874..64d09f2e75 100644
--- a/linux-user/i386/target_signal.h
+++ b/linux-user/i386/target_signal.h
@@ -22,4 +22,6 @@  typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* I386_TARGET_SIGNAL_H */
diff --git a/linux-user/x86_64/target_signal.h b/linux-user/x86_64/target_signal.h
index 4ea74f20dd..4673c5a886 100644
--- a/linux-user/x86_64/target_signal.h
+++ b/linux-user/x86_64/target_signal.h
@@ -21,4 +21,7 @@  typedef struct target_sigaltstack {
 
 #include "../generic/signal.h"
 
+/* For x86_64, use of SA_RESTORER is mandatory. */
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
+
 #endif /* X86_64_TARGET_SIGNAL_H */
diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index 8701774e37..a83ecba54f 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -337,16 +337,7 @@  void setup_frame(int sig, struct target_sigaction *ka,
     if (ka->sa_flags & TARGET_SA_RESTORER) {
         __put_user(ka->sa_restorer, &frame->pretcode);
     } else {
-        uint16_t val16;
-        abi_ulong retcode_addr;
-        retcode_addr = frame_addr + offsetof(struct sigframe, retcode);
-        __put_user(retcode_addr, &frame->pretcode);
-        /* This is popl %eax ; movl $,%eax ; int $0x80 */
-        val16 = 0xb858;
-        __put_user(val16, (uint16_t *)(frame->retcode+0));
-        __put_user(TARGET_NR_sigreturn, (int *)(frame->retcode+2));
-        val16 = 0x80cd;
-        __put_user(val16, (uint16_t *)(frame->retcode+6));
+        __put_user(default_sigreturn, &frame->pretcode);
     }
 
     /* Set up registers for signal handler */
@@ -415,14 +406,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     if (ka->sa_flags & TARGET_SA_RESTORER) {
         __put_user(ka->sa_restorer, &frame->pretcode);
     } else {
-        uint16_t val16;
-        addr = frame_addr + offsetof(struct rt_sigframe, retcode);
-        __put_user(addr, &frame->pretcode);
-        /* This is movl $,%eax ; int $0x80 */
-        __put_user(0xb8, (char *)(frame->retcode+0));
-        __put_user(TARGET_NR_rt_sigreturn, (int *)(frame->retcode+1));
-        val16 = 0x80cd;
-        __put_user(val16, (uint16_t *)(frame->retcode+5));
+        __put_user(default_rt_sigreturn, &frame->pretcode);
     }
 #else
     /* XXX: Would be slightly better to return -EFAULT here if test fails
@@ -591,3 +575,25 @@  badframe:
     force_sig(TARGET_SIGSEGV);
     return -TARGET_QEMU_ESIGRETURN;
 }
+
+#ifndef TARGET_X86_64
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+    uint16_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, 2 * 8, 0);
+    assert(tramp != NULL);
+
+    default_sigreturn = sigtramp_page;
+    /* This is popl %eax ; movl $,%eax ; int $0x80 */
+    __put_user(0xb858, (uint16_t *)(tramp + 0));
+    __put_user(TARGET_NR_sigreturn, (int *)(tramp + 2));
+    __put_user(0x80cd, (uint16_t *)(tramp + 6));
+
+    default_rt_sigreturn = sigtramp_page + 8;
+    /* This is movl $,%eax ; int $0x80 */
+    __put_user(0xb8, (char *)(tramp + 8));
+    __put_user(TARGET_NR_rt_sigreturn, (int *)(tramp + 9));
+    __put_user(0x80cd, (uint16_t *)(tramp + 13));
+
+    unlock_user(tramp, sigtramp_page, 2 * 8);
+}
+#endif