Skip to content
This repository was archived by the owner on Oct 18, 2024. It is now read-only.

Commit 84fc148

Browse files
zatrazzchiichen
zatrazz
authored and
chiichen
committed
sparc: Fix restartable syscalls (BZ 32173)
The commit 'sparc: Use Linux kABI for syscall return' (86c5d2c) did not take into account a subtle sparc syscall kABI constraint. For syscalls that might block indefinitely, on an interrupt (like SIGCONT) the kernel will set the instruction pointer to just before the syscall: arch/sparc/kernel/signal_64.c 476 static void do_signal(struct pt_regs *regs, unsigned long orig_i0) 477 { [...] 525 if (restart_syscall) { 526 switch (regs->u_regs[UREG_I0]) { 527 case ERESTARTNOHAND: 528 case ERESTARTSYS: 529 case ERESTARTNOINTR: 530 /* replay the system call when we are done */ 531 regs->u_regs[UREG_I0] = orig_i0; 532 regs->tpc -= 4; 533 regs->tnpc -= 4; 534 pt_regs_clear_syscall(regs); 535 fallthrough; 536 case ERESTART_RESTARTBLOCK: 537 regs->u_regs[UREG_G1] = __NR_restart_syscall; 538 regs->tpc -= 4; 539 regs->tnpc -= 4; 540 pt_regs_clear_syscall(regs); 541 } However, on a SIGCONT it seems that 'g1' register is being clobbered after the syscall returns. Before 86c5d2c, the 'g1' was always placed jus before the 'ta' instruction which then reloads the syscall number and restarts the syscall. On master, where 'g1' might be placed before 'ta': $ cat test.c #include <unistd.h> int main () { pause (); } $ gcc test.c -o test $ strace -f ./t [...] ppoll(NULL, 0, NULL, NULL, 0 On another terminal $ kill -STOP 2262828 $ strace -f ./t [...] --- SIGSTOP {si_signo=SIGSTOP, si_code=SI_USER, si_pid=2521813, si_uid=8289} --- --- stopped by SIGSTOP --- And then $ kill -CONT 2262828 Results in: --- SIGCONT {si_signo=SIGCONT, si_code=SI_USER, si_pid=2521813, si_uid=8289} --- restart_syscall(<... resuming interrupted ppoll ...>) = -1 EINTR (Interrupted system call) Where the expected behaviour would be: $ strace -f ./t [...] ppoll(NULL, 0, NULL, NULL, 0) = ? ERESTARTNOHAND (To be restarted if no handler) --- SIGSTOP {si_signo=SIGSTOP, si_code=SI_USER, si_pid=2521813, si_uid=8289} --- --- stopped by SIGSTOP --- --- SIGCONT {si_signo=SIGCONT, si_code=SI_USER, si_pid=2521813, si_uid=8289} --- ppoll(NULL, 0, NULL, NULL, 0 Just moving the 'g1' setting near the syscall asm is not suffice, the compiler might optimize it away (as I saw on cancellation.c by trying this fix). Instead, I have change the inline asm to put the 'g1' setup in ithe asm block. This would require to change the asm constraint for INTERNAL_SYSCALL_NCS, since the syscall number is not constant. Checked on sparc64-linux-gnu. Reported-by: René Rebe <[email protected]> Tested-by: Sam James <[email protected]> Reviewed-by: Sam James <[email protected]>
1 parent b3a6a83 commit 84fc148

File tree

7 files changed

+165
-36
lines changed

7 files changed

+165
-36
lines changed

‎sysdeps/unix/sysv/linux/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ tests += \
230230
tst-scm_rights \
231231
tst-sigtimedwait \
232232
tst-sync_file_range \
233+
tst-syscall-restart \
233234
tst-sysconf-iov_max \
234235
tst-sysvmsg-linux \
235236
tst-sysvsem-linux \

‎sysdeps/unix/sysv/linux/sparc/sparc32/syscall_cancel.S

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ __syscall_cancel_arch_start:
4848
bne 2f
4949
#endif
5050
/* Issue a 6 argument syscall. */
51-
mov %i1, %g1
52-
mov %i2, %o0
51+
mov %i2, %o0
5352
mov %i3, %o1
5453
mov %i4, %o2
5554
mov %i5, %o3
5655
ld [%fp+92], %o4
5756
ld [%fp+96], %o5
57+
mov %i1, %g1
5858
ta 0x10
5959

6060
.globl __syscall_cancel_arch_end

‎sysdeps/unix/sysv/linux/sparc/sparc32/sysdep.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,15 @@ ENTRY(name); \
107107
#else /* __ASSEMBLER__ */
108108

109109
#define __SYSCALL_STRING \
110+
"mov %[scn], %%g1;" \
110111
"ta 0x10;" \
111112
"bcc 1f;" \
112113
" nop;" \
113114
"sub %%g0, %%o0, %%o0;" \
114115
"1:"
115116

116117
#define __SYSCALL_CLOBBERS \
117-
"f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", \
118+
"g1", "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", \
118119
"f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15", \
119120
"f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23", \
120121
"f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", \

‎sysdeps/unix/sysv/linux/sparc/sparc64/syscall_cancel.S

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ __syscall_cancel_arch_start:
4646
andcc %g2, TCB_CANCELED_BITMASK, %g0
4747
bne,pn %xcc, 2f
4848
/* Issue a 6 argument syscall. */
49-
mov %i1, %g1
50-
mov %i2, %o0
49+
mov %i2, %o0
5150
mov %i3, %o1
5251
mov %i4, %o2
5352
mov %i5, %o3
5453
ldx [%fp + STACK_BIAS + 176], %o4
5554
ldx [%fp + STACK_BIAS + 184], %o5
55+
mov %i1, %g1
5656
ta 0x6d
5757

5858
.global __syscall_cancel_arch_end

‎sysdeps/unix/sysv/linux/sparc/sparc64/sysdep.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,15 @@ ENTRY(name); \
106106
#else /* __ASSEMBLER__ */
107107

108108
#define __SYSCALL_STRING \
109+
"mov %[scn], %%g1;" \
109110
"ta 0x6d;" \
110111
"bcc,pt %%xcc, 1f;" \
111112
" nop;" \
112113
"sub %%g0, %%o0, %%o0;" \
113114
"1:"
114115

115116
#define __SYSCALL_CLOBBERS \
116-
"f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", \
117+
"g1", "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", \
117118
"f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15", \
118119
"f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23", \
119120
"f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", \

‎sysdeps/unix/sysv/linux/sparc/sysdep.h

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,118 +50,132 @@
5050

5151
#undef INTERNAL_SYSCALL_NCS
5252
#define INTERNAL_SYSCALL_NCS(name, nr, args...) \
53-
internal_syscall##nr(__SYSCALL_STRING, name, args)
53+
_internal_syscall##nr(__SYSCALL_STRING, "p", name, args)
5454

55-
#define internal_syscall0(string,name,dummy...) \
55+
#define _internal_syscall0(string,nc,name,dummy...) \
5656
({ \
57-
register long int __g1 __asm__ ("g1") = (name); \
5857
register long __o0 __asm__ ("o0"); \
58+
long int _name = (long int) (name); \
5959
__asm __volatile (string : "=r" (__o0) : \
60-
"r" (__g1) : \
60+
[scn] nc (_name) : \
6161
__SYSCALL_CLOBBERS); \
6262
__o0; \
6363
})
64+
#define internal_syscall0(string,name,args...) \
65+
_internal_syscall0(string, "i", name, args)
6466

65-
#define internal_syscall1(string,name,arg1) \
67+
#define _internal_syscall1(string,nc,name,arg1) \
6668
({ \
6769
long int _arg1 = (long int) (arg1); \
68-
register long int __g1 __asm__("g1") = (name); \
70+
long int _name = (long int) (name); \
6971
register long int __o0 __asm__ ("o0") = _arg1; \
70-
__asm __volatile (string : "=r" (__o0) : \
71-
"r" (__g1), "0" (__o0) : \
72+
__asm __volatile (string : "+r" (__o0) : \
73+
[scn] nc (_name) : \
7274
__SYSCALL_CLOBBERS); \
7375
__o0; \
7476
})
77+
#define internal_syscall1(string,name,args...) \
78+
_internal_syscall1(string, "i", name, args)
7579

76-
#define internal_syscall2(string,name,arg1,arg2) \
80+
#define _internal_syscall2(string,nc,name,arg1,arg2) \
7781
({ \
7882
long int _arg1 = (long int) (arg1); \
7983
long int _arg2 = (long int) (arg2); \
80-
register long int __g1 __asm__("g1") = (name); \
84+
long int _name = (long int) (name); \
8185
register long int __o0 __asm__ ("o0") = _arg1; \
8286
register long int __o1 __asm__ ("o1") = _arg2; \
83-
__asm __volatile (string : "=r" (__o0) : \
84-
"r" (__g1), "0" (__o0), "r" (__o1) : \
87+
__asm __volatile (string : "+r" (__o0) : \
88+
[scn] nc (_name), "r" (__o1) : \
8589
__SYSCALL_CLOBBERS); \
8690
__o0; \
8791
})
92+
#define internal_syscall2(string,name,args...) \
93+
_internal_syscall2(string, "i", name, args)
8894

89-
#define internal_syscall3(string,name,arg1,arg2,arg3) \
95+
#define _internal_syscall3(string,nc,name,arg1,arg2,arg3) \
9096
({ \
9197
long int _arg1 = (long int) (arg1); \
9298
long int _arg2 = (long int) (arg2); \
9399
long int _arg3 = (long int) (arg3); \
94-
register long int __g1 __asm__("g1") = (name); \
100+
long int _name = (long int) (name); \
95101
register long int __o0 __asm__ ("o0") = _arg1; \
96102
register long int __o1 __asm__ ("o1") = _arg2; \
97103
register long int __o2 __asm__ ("o2") = _arg3; \
98-
__asm __volatile (string : "=r" (__o0) : \
99-
"r" (__g1), "0" (__o0), "r" (__o1), \
104+
__asm __volatile (string : "+r" (__o0) : \
105+
[scn] nc (_name), "r" (__o1), \
100106
"r" (__o2) : \
101107
__SYSCALL_CLOBBERS); \
102108
__o0; \
103109
})
110+
#define internal_syscall3(string,name,args...) \
111+
_internal_syscall3(string, "i", name, args)
104112

105-
#define internal_syscall4(string,name,arg1,arg2,arg3,arg4) \
113+
#define _internal_syscall4(string,nc,name,arg1,arg2,arg3,arg4) \
106114
({ \
107115
long int _arg1 = (long int) (arg1); \
108116
long int _arg2 = (long int) (arg2); \
109117
long int _arg3 = (long int) (arg3); \
110118
long int _arg4 = (long int) (arg4); \
111-
register long int __g1 __asm__("g1") = (name); \
119+
long int _name = (long int) (name); \
112120
register long int __o0 __asm__ ("o0") = _arg1; \
113121
register long int __o1 __asm__ ("o1") = _arg2; \
114122
register long int __o2 __asm__ ("o2") = _arg3; \
115123
register long int __o3 __asm__ ("o3") = _arg4; \
116-
__asm __volatile (string : "=r" (__o0) : \
117-
"r" (__g1), "0" (__o0), "r" (__o1), \
124+
__asm __volatile (string : "+r" (__o0) : \
125+
[scn] nc (_name), "r" (__o1), \
118126
"r" (__o2), "r" (__o3) : \
119127
__SYSCALL_CLOBBERS); \
120128
__o0; \
121129
})
130+
#define internal_syscall4(string,name,args...) \
131+
_internal_syscall4(string, "i", name, args)
122132

123-
#define internal_syscall5(string,name,arg1,arg2,arg3,arg4,arg5) \
133+
#define _internal_syscall5(string,nc,name,arg1,arg2,arg3,arg4,arg5) \
124134
({ \
125135
long int _arg1 = (long int) (arg1); \
126136
long int _arg2 = (long int) (arg2); \
127137
long int _arg3 = (long int) (arg3); \
128138
long int _arg4 = (long int) (arg4); \
129139
long int _arg5 = (long int) (arg5); \
130-
register long int __g1 __asm__("g1") = (name); \
140+
long int _name = (long int) (name); \
131141
register long int __o0 __asm__ ("o0") = _arg1; \
132142
register long int __o1 __asm__ ("o1") = _arg2; \
133143
register long int __o2 __asm__ ("o2") = _arg3; \
134144
register long int __o3 __asm__ ("o3") = _arg4; \
135145
register long int __o4 __asm__ ("o4") = _arg5; \
136-
__asm __volatile (string : "=r" (__o0) : \
137-
"r" (__g1), "0" (__o0), "r" (__o1), \
146+
__asm __volatile (string : "+r" (__o0) : \
147+
[scn] nc (_name), "r" (__o1), \
138148
"r" (__o2), "r" (__o3), "r" (__o4) : \
139149
__SYSCALL_CLOBBERS); \
140150
__o0; \
141151
})
152+
#define internal_syscall5(string,name,args...) \
153+
_internal_syscall5(string, "i", name, args)
142154

143-
#define internal_syscall6(string,name,arg1,arg2,arg3,arg4,arg5,arg6) \
155+
#define _internal_syscall6(string,nc,name,arg1,arg2,arg3,arg4,arg5,arg6)\
144156
({ \
145157
long int _arg1 = (long int) (arg1); \
146158
long int _arg2 = (long int) (arg2); \
147159
long int _arg3 = (long int) (arg3); \
148160
long int _arg4 = (long int) (arg4); \
149161
long int _arg5 = (long int) (arg5); \
150162
long int _arg6 = (long int) (arg6); \
151-
register long int __g1 __asm__("g1") = (name); \
163+
long int _name = (long int) (name); \
152164
register long int __o0 __asm__ ("o0") = _arg1; \
153165
register long int __o1 __asm__ ("o1") = _arg2; \
154166
register long int __o2 __asm__ ("o2") = _arg3; \
155167
register long int __o3 __asm__ ("o3") = _arg4; \
156168
register long int __o4 __asm__ ("o4") = _arg5; \
157169
register long int __o5 __asm__ ("o5") = _arg6; \
158-
__asm __volatile (string : "=r" (__o0) : \
159-
"r" (__g1), "0" (__o0), "r" (__o1), \
170+
__asm __volatile (string : "+r" (__o0) : \
171+
[scn] nc (_name), "r" (__o1), \
160172
"r" (__o2), "r" (__o3), "r" (__o4), \
161173
"r" (__o5) : \
162174
__SYSCALL_CLOBBERS); \
163175
__o0; \
164176
})
177+
#define internal_syscall6(string,name,args...) \
178+
_internal_syscall6(string, "i", name, args)
165179

166180
#define INLINE_CLONE_SYSCALL(arg1,arg2,arg3,arg4,arg5) \
167181
({ \
@@ -170,15 +184,15 @@
170184
long int _arg3 = (long int) (arg3); \
171185
long int _arg4 = (long int) (arg4); \
172186
long int _arg5 = (long int) (arg5); \
187+
long int _name = __NR_clone; \
173188
register long int __o0 __asm__ ("o0") = _arg1; \
174189
register long int __o1 __asm__ ("o1") = _arg2; \
175190
register long int __o2 __asm__ ("o2") = _arg3; \
176191
register long int __o3 __asm__ ("o3") = _arg4; \
177192
register long int __o4 __asm__ ("o4") = _arg5; \
178-
register long int __g1 __asm__ ("g1") = __NR_clone; \
179193
__asm __volatile (__SYSCALL_STRING : \
180194
"=r" (__o0), "=r" (__o1) : \
181-
"r" (__g1), "0" (__o0), "1" (__o1), \
195+
[scn] "i" (_name), "0" (__o0), "1" (__o1), \
182196
"r" (__o2), "r" (__o3), "r" (__o4) : \
183197
__SYSCALL_CLOBBERS); \
184198
if (__glibc_unlikely ((unsigned long int) (__o0) > -4096UL)) \

‎sysdeps/unix/sysv/linux/tst-syscall-restart.c

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/* Test if a syscall is correctly restarted.
2+
Copyright (C) 2024 Free Software Foundation, Inc.
3+
This file is part of the GNU C Library.
4+
5+
The GNU C Library is free software; you can redistribute it and/or
6+
modify it under the terms of the GNU Lesser General Public
7+
License as published by the Free Software Foundation; either
8+
version 2.1 of the License, or (at your option) any later version.
9+
10+
The GNU C Library is distributed in the hope that it will be useful,
11+
but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
Lesser General Public License for more details.
14+
15+
You should have received a copy of the GNU Lesser General Public
16+
License along with the GNU C Library; if not, see
17+
<https://www.gnu.org/licenses/>. */
18+
19+
#include <support/xsignal.h>
20+
#include <support/check.h>
21+
#include <support/process_state.h>
22+
#include <support/xunistd.h>
23+
#include <support/xthread.h>
24+
#include <sys/wait.h>
25+
26+
static int
27+
check_pid (pid_t pid)
28+
{
29+
/* Wait until the child has called pause and it blocking on kernel. */
30+
support_process_state_wait (pid, support_process_state_sleeping);
31+
32+
TEST_COMPARE (kill (pid, SIGSTOP), 0);
33+
34+
/* Adding process_state_tracing_stop ('t') allows the test to work under
35+
trace programs such as ptrace. */
36+
support_process_state_wait (pid, support_process_state_stopped
37+
| support_process_state_tracing_stop);
38+
39+
TEST_COMPARE (kill (pid, SIGCONT), 0);
40+
41+
enum support_process_state state
42+
= support_process_state_wait (pid, support_process_state_sleeping
43+
| support_process_state_zombie);
44+
45+
TEST_COMPARE (state, support_process_state_sleeping);
46+
47+
TEST_COMPARE (kill (pid, SIGTERM), 0);
48+
49+
siginfo_t info;
50+
TEST_COMPARE (waitid (P_PID, pid, &info, WEXITED), 0);
51+
TEST_COMPARE (info.si_signo, SIGCHLD);
52+
TEST_COMPARE (info.si_code, CLD_KILLED);
53+
TEST_COMPARE (info.si_status, SIGTERM);
54+
TEST_COMPARE (info.si_pid, pid);
55+
56+
return 0;
57+
}
58+
59+
static void *
60+
tf (void *)
61+
{
62+
pause ();
63+
return NULL;
64+
}
65+
66+
static void
67+
child_mt (void)
68+
{
69+
/* Let only the created thread to handle signals. */
70+
sigset_t set;
71+
sigfillset (&set);
72+
xpthread_sigmask (SIG_BLOCK, &set, NULL);
73+
74+
sigdelset (&set, SIGSTOP);
75+
sigdelset (&set, SIGCONT);
76+
sigdelset (&set, SIGTERM);
77+
78+
pthread_attr_t attr;
79+
xpthread_attr_init (&attr);
80+
TEST_COMPARE (pthread_attr_setsigmask_np (&attr, &set), 0);
81+
82+
xpthread_join (xpthread_create (&attr, tf, NULL));
83+
}
84+
85+
static void
86+
do_test_syscall (bool multithread)
87+
{
88+
pid_t pid = xfork ();
89+
if (pid == 0)
90+
{
91+
if (multithread)
92+
child_mt ();
93+
else
94+
pause ();
95+
_exit (127);
96+
}
97+
98+
check_pid (pid);
99+
}
100+
101+
static int
102+
do_test (void)
103+
{
104+
/* Check for both single and multi thread, since they use different syscall
105+
mechanisms. */
106+
do_test_syscall (false);
107+
do_test_syscall (true);
108+
109+
return 0;
110+
}
111+
112+
#include <support/test-driver.c>

0 commit comments

Comments
 (0)