* [PATCH 0/4] Unblocked by fake_signal_wake_up then real signal
@ 2018-03-20 2:38 NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 1/4] signal/x86: Factor out nosig handling NIIBE Yutaka
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: NIIBE Yutaka @ 2018-03-20 2:38 UTC (permalink / raw)
To: linux-kernel; +Cc: gniibe
Hello,
Background:
I tried to fix my own problem in suspend-to-ram with USB devices (some
device doesn't work well after suspend/resume). I know it's common
problems. Well, while I identified some problems of USB devices and
drivers, I did try my best. I fixed a USB device implementation of
mine. Then, I also located and fixed bugs in a USB user space driver
which I maintain.
Now, I think I found a kernel problem. After suspend/resume, in my
USB user space driver, a task sends a signal to another task, then,
pselect is unblocked by the signal. In this situation, a signal
handler is called correctly, but it seems that the pselect is
restarted. Expected behavior is unblocked pselect returning -EINTR.
Looking the code, my theory is:
It is a fake signal by suspend which unblocks pselect to be frozen,
and after resume, real signal is sent. When both of signals are
handled in a single call of exit_to_usermode_loop, by two calls of
do_signal, firstly for fake signal and secondly for real signal, the
system call is restarted wrongly, where it should return -EINTR.
Here is a patch series of mine for x86. I'd suggest architecture
maintainers to do similar if an architecture supports suspend/resume.
NIIBE Yutaka (4):
signal/x86: Factor out nosig handling.
signal/x86: do_signal: syscall restart should be done only once.
signal/x86: Move nosig handling at the end of exit_to_usermode_loop.
signal/x86: Move restore_saved_sigmask().
arch/x86/entry/common.c | 6 +++++-
arch/x86/include/asm/signal.h | 3 ++-
arch/x86/kernel/signal.c | 22 ++++++++++++++--------
3 files changed, 21 insertions(+), 10 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/4] signal/x86: Factor out nosig handling.
2018-03-20 2:38 [PATCH 0/4] Unblocked by fake_signal_wake_up then real signal NIIBE Yutaka
@ 2018-03-20 2:38 ` NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 2/4] signal/x86: do_signal: syscall restart should be done only once NIIBE Yutaka
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: NIIBE Yutaka @ 2018-03-20 2:38 UTC (permalink / raw)
To: linux-kernel; +Cc: gniibe
Make a function for swing at the ball and miss.
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
---
arch/x86/include/asm/signal.h | 1 +
arch/x86/kernel/signal.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/arch/x86/include/asm/signal.h b/arch/x86/include/asm/signal.h
index 5f9012ff52ed..6517df6d9938 100644
--- a/arch/x86/include/asm/signal.h
+++ b/arch/x86/include/asm/signal.h
@@ -36,6 +36,7 @@ typedef sigset_t compat_sigset_t;
#include <uapi/asm/signal.h>
#ifndef __ASSEMBLY__
extern void do_signal(struct pt_regs *regs);
+extern void nosig_restart_syscall(struct pt_regs *regs);
#define __ARCH_HAS_SA_RESTORER
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 4cdc0b27ec82..cdfb82031243 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -812,6 +812,11 @@ void do_signal(struct pt_regs *regs)
return;
}
+ nosig_restart_syscall(regs);
+}
+
+void nosig_restart_syscall(struct pt_regs *regs)
+{
/* Did we come from a system call? */
if (syscall_get_nr(current, regs) >= 0) {
/* Restart the system call - no handlers present */
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] signal/x86: do_signal: syscall restart should be done only once.
2018-03-20 2:38 [PATCH 0/4] Unblocked by fake_signal_wake_up then real signal NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 1/4] signal/x86: Factor out nosig handling NIIBE Yutaka
@ 2018-03-20 2:38 ` NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 3/4] signal/x86: Move nosig handling at the end of exit_to_usermode_loop NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 4/4] signal/x86: Move restore_saved_sigmask() NIIBE Yutaka
3 siblings, 0 replies; 6+ messages in thread
From: NIIBE Yutaka @ 2018-03-20 2:38 UTC (permalink / raw)
To: linux-kernel; +Cc: gniibe
do_signal may be called multiple times from exit_to_usermode_loop. In
those multiple calls, only the one should handle restarting the system
call.
When actually delivering a signal, make sure the register will not be
examined again as syscall errno by another call of do_signal.
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
---
arch/x86/kernel/signal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cdfb82031243..e709b54a01b8 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -729,6 +729,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
regs->ip -= 2;
break;
}
+ regs->orig_ax = -1;
}
/*
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] signal/x86: Move nosig handling at the end of exit_to_usermode_loop.
2018-03-20 2:38 [PATCH 0/4] Unblocked by fake_signal_wake_up then real signal NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 1/4] signal/x86: Factor out nosig handling NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 2/4] signal/x86: do_signal: syscall restart should be done only once NIIBE Yutaka
@ 2018-03-20 2:38 ` NIIBE Yutaka
2018-03-21 1:39 ` kbuild test robot
2018-03-20 2:38 ` [PATCH 4/4] signal/x86: Move restore_saved_sigmask() NIIBE Yutaka
3 siblings, 1 reply; 6+ messages in thread
From: NIIBE Yutaka @ 2018-03-20 2:38 UTC (permalink / raw)
To: linux-kernel; +Cc: gniibe
Handling of restarting a system call for nosig should be done after
all possible calls of do_signal, to see if it's actually delivering
signal(s) or not.
Before this change, restarting a system call for nosig may be followed
by a call of handle_signal which would try to change the system call
returning -EINTR instead, but it is too late to do so.
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
---
arch/x86/entry/common.c | 6 +++++-
arch/x86/include/asm/signal.h | 2 +-
arch/x86/kernel/signal.c | 6 +++---
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 74f6eee15179..38e0939c0aeb 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -137,6 +137,8 @@ static long syscall_trace_enter(struct pt_regs *regs)
static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
{
+ int nosig = 0;
+
/*
* In order to return to user mode, we need to have IRQs off with
* none of EXIT_TO_USERMODE_LOOP_FLAGS set. Several of these flags
@@ -159,7 +161,7 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
/* deal with pending signal delivery */
if (cached_flags & _TIF_SIGPENDING)
- do_signal(regs);
+ nosig |= do_signal(regs);
if (cached_flags & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
@@ -177,6 +179,8 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
if (!(cached_flags & EXIT_TO_USERMODE_LOOP_FLAGS))
break;
}
+ if (nosig)
+ nosig_restart_syscall(regs);
}
/* Called with IRQs disabled. */
diff --git a/arch/x86/include/asm/signal.h b/arch/x86/include/asm/signal.h
index 6517df6d9938..31e68266c2f6 100644
--- a/arch/x86/include/asm/signal.h
+++ b/arch/x86/include/asm/signal.h
@@ -35,7 +35,7 @@ typedef sigset_t compat_sigset_t;
#endif /* __ASSEMBLY__ */
#include <uapi/asm/signal.h>
#ifndef __ASSEMBLY__
-extern void do_signal(struct pt_regs *regs);
+extern int do_signal(struct pt_regs *regs);
extern void nosig_restart_syscall(struct pt_regs *regs);
#define __ARCH_HAS_SA_RESTORER
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index e709b54a01b8..57576988a01f 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -803,17 +803,17 @@ static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
* want to handle. Thus you cannot kill init even with a SIGKILL even by
* mistake.
*/
-void do_signal(struct pt_regs *regs)
+int do_signal(struct pt_regs *regs)
{
struct ksignal ksig;
if (get_signal(&ksig)) {
/* Whee! Actually deliver the signal. */
handle_signal(&ksig, regs);
- return;
+ return 0;
}
- nosig_restart_syscall(regs);
+ return 1;
}
void nosig_restart_syscall(struct pt_regs *regs)
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/4] signal/x86: Move nosig handling at the end of exit_to_usermode_loop.
2018-03-20 2:38 ` [PATCH 3/4] signal/x86: Move nosig handling at the end of exit_to_usermode_loop NIIBE Yutaka
@ 2018-03-21 1:39 ` kbuild test robot
0 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2018-03-21 1:39 UTC (permalink / raw)
To: NIIBE Yutaka; +Cc: kbuild-all, linux-kernel, gniibe
[-- Attachment #1: Type: text/plain, Size: 3540 bytes --]
Hi NIIBE,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180320]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/NIIBE-Yutaka/Unblocked-by-fake_signal_wake_up-then-real-signal/20180321-051147
config: um-x86_64_defconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=um SUBARCH=x86_64
All errors (new ones prefixed by >>):
In file included from arch/um/kernel/irq.c:17:0:
>> arch/um/include/shared/kern_util.h:26:13: error: conflicting types for 'do_signal'
extern void do_signal(struct pt_regs *regs);
^~~~~~~~~
In file included from include/uapi/linux/signal.h:5:0,
from include/linux/signal_types.h:10,
from include/linux/sched.h:27,
from include/linux/kernel_stat.h:10,
from arch/um/kernel/irq.c:11:
arch/x86/include/asm/signal.h:38:12: note: previous declaration of 'do_signal' was here
extern int do_signal(struct pt_regs *regs);
^~~~~~~~~
--
In file included from arch/um/kernel/signal.c:13:0:
>> arch/um/include/shared/kern_util.h:26:13: error: conflicting types for 'do_signal'
extern void do_signal(struct pt_regs *regs);
^~~~~~~~~
In file included from include/uapi/linux/signal.h:5:0,
from include/linux/signal_types.h:10,
from include/linux/sched.h:27,
from include/linux/ptrace.h:6,
from arch/um/kernel/signal.c:7:
arch/x86/include/asm/signal.h:38:12: note: previous declaration of 'do_signal' was here
extern int do_signal(struct pt_regs *regs);
^~~~~~~~~
>> arch/um/kernel/signal.c:67:6: error: conflicting types for 'do_signal'
void do_signal(struct pt_regs *regs)
^~~~~~~~~
In file included from include/uapi/linux/signal.h:5:0,
from include/linux/signal_types.h:10,
from include/linux/sched.h:27,
from include/linux/ptrace.h:6,
from arch/um/kernel/signal.c:7:
arch/x86/include/asm/signal.h:38:12: note: previous declaration of 'do_signal' was here
extern int do_signal(struct pt_regs *regs);
^~~~~~~~~
vim +/do_signal +26 arch/um/include/shared/kern_util.h
edea13858 arch/um/include/kern_util.h Jeff Dike 2008-02-04 24
ccaee5f85 arch/um/include/shared/kern_util.h Ingo Molnar 2015-07-03 25 struct pt_regs;
ccaee5f85 arch/um/include/shared/kern_util.h Ingo Molnar 2015-07-03 @26 extern void do_signal(struct pt_regs *regs);
edea13858 arch/um/include/kern_util.h Jeff Dike 2008-02-04 27 extern void interrupt_end(void);
d3c1cfcdb arch/um/include/shared/kern_util.h Martin Pärtel 2012-08-02 28 extern void relay_signal(int sig, struct siginfo *si, struct uml_pt_regs *regs);
edea13858 arch/um/include/kern_util.h Jeff Dike 2008-02-04 29
:::::: The code at line 26 was first introduced by commit
:::::: ccaee5f851470dec6894a6835b6fadffc2bb7514 um: Fix do_signal() prototype
:::::: TO: Ingo Molnar <mingo@kernel.org>
:::::: CC: Ingo Molnar <mingo@kernel.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8114 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/4] signal/x86: Move restore_saved_sigmask().
2018-03-20 2:38 [PATCH 0/4] Unblocked by fake_signal_wake_up then real signal NIIBE Yutaka
` (2 preceding siblings ...)
2018-03-20 2:38 ` [PATCH 3/4] signal/x86: Move nosig handling at the end of exit_to_usermode_loop NIIBE Yutaka
@ 2018-03-20 2:38 ` NIIBE Yutaka
3 siblings, 0 replies; 6+ messages in thread
From: NIIBE Yutaka @ 2018-03-20 2:38 UTC (permalink / raw)
To: linux-kernel; +Cc: gniibe
Call to restore_saved_sigmask is only needed when it is from a system
call. It is only unblocked system call which uses saved_sigmask.
Specifically, they are pselect, ppoll, and epoll_pwait.
Signed-off-by: NIIBE Yutaka <gniibe@fsij.org>
---
arch/x86/kernel/signal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 57576988a01f..c3928718a33a 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -834,13 +834,13 @@ void nosig_restart_syscall(struct pt_regs *regs)
regs->ip -= 2;
break;
}
- }
- /*
- * If there's no signal to deliver, we just put the saved sigmask
- * back.
- */
- restore_saved_sigmask();
+ /*
+ * If there's no signal to deliver, we just put the
+ * saved sigmask back.
+ */
+ restore_saved_sigmask();
+ }
}
void signal_fault(struct pt_regs *regs, void __user *frame, char *where)
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-21 1:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-20 2:38 [PATCH 0/4] Unblocked by fake_signal_wake_up then real signal NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 1/4] signal/x86: Factor out nosig handling NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 2/4] signal/x86: do_signal: syscall restart should be done only once NIIBE Yutaka
2018-03-20 2:38 ` [PATCH 3/4] signal/x86: Move nosig handling at the end of exit_to_usermode_loop NIIBE Yutaka
2018-03-21 1:39 ` kbuild test robot
2018-03-20 2:38 ` [PATCH 4/4] signal/x86: Move restore_saved_sigmask() NIIBE Yutaka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).