diff mbox series

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

Message ID 20210618192951.125651-11-richard.henderson@linaro.org
State New
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(-)

-- 
2.25.1

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