Skip to content

Commit 33da8e7

Browse files
committed
signal: Allow cifs and drbd to receive their terminating signals
My recent to change to only use force_sig for a synchronous events wound up breaking signal reception cifs and drbd. I had overlooked the fact that by default kthreads start out with all signals set to SIG_IGN. So a change I thought was safe turned out to have made it impossible for those kernel thread to catch their signals. Reverting the work on force_sig is a bad idea because what the code was doing was very much a misuse of force_sig. As the way force_sig ultimately allowed the signal to happen was to change the signal handler to SIG_DFL. Which after the first signal will allow userspace to send signals to these kernel threads. At least for wake_ack_receiver in drbd that does not appear actively wrong. So correct this problem by adding allow_kernel_signal that will allow signals whose siginfo reports they were sent by the kernel through, but will not allow userspace generated signals, and update cifs and drbd to call allow_kernel_signal in an appropriate place so that their thread can receive this signal. Fixing things this way ensures that userspace won't be able to send signals and cause problems, that it is clear which signals the threads are expecting to receive, and it guarantees that nothing else in the system will be affected. This change was partly inspired by similar cifs and drbd patches that added allow_signal. Reported-by: ronnie sahlberg <[email protected]> Reported-by: Christoph Böhmwalder <[email protected]> Tested-by: Christoph Böhmwalder <[email protected]> Cc: Steve French <[email protected]> Cc: Philipp Reisner <[email protected]> Cc: David Laight <[email protected]> Fixes: 247bc94 ("cifs: fix rmmod regression in cifs.ko caused by force_sig changes") Fixes: 72abe3b ("signal/cifs: Fix cifs_put_tcp_session to call send_sig instead of force_sig") Fixes: fee1099 ("signal/drbd: Use send_sig not force_sig") Fixes: 3cf5d07 ("signal: Remove task parameter from force_sig") Signed-off-by: "Eric W. Biederman" <[email protected]>
1 parent d45331b commit 33da8e7

File tree

4 files changed

+22
-2
lines changed

4 files changed

+22
-2
lines changed

drivers/block/drbd/drbd_main.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,8 @@ static int drbd_thread_setup(void *arg)
322322
thi->name[0],
323323
resource->name);
324324

325+
allow_kernel_signal(DRBD_SIGKILL);
326+
allow_kernel_signal(SIGXCPU);
325327
restart:
326328
retval = thi->function(thi);
327329

fs/cifs/connect.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1113,7 +1113,7 @@ cifs_demultiplex_thread(void *p)
11131113
mempool_resize(cifs_req_poolp, length + cifs_min_rcv);
11141114

11151115
set_freezable();
1116-
allow_signal(SIGKILL);
1116+
allow_kernel_signal(SIGKILL);
11171117
while (server->tcpStatus != CifsExiting) {
11181118
if (try_to_freeze())
11191119
continue;

include/linux/signal.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,14 +282,27 @@ extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
282282
extern void exit_signals(struct task_struct *tsk);
283283
extern void kernel_sigaction(int, __sighandler_t);
284284

285+
#define SIG_KTHREAD ((__force __sighandler_t)2)
286+
#define SIG_KTHREAD_KERNEL ((__force __sighandler_t)3)
287+
285288
static inline void allow_signal(int sig)
286289
{
287290
/*
288291
* Kernel threads handle their own signals. Let the signal code
289292
* know it'll be handled, so that they don't get converted to
290293
* SIGKILL or just silently dropped.
291294
*/
292-
kernel_sigaction(sig, (__force __sighandler_t)2);
295+
kernel_sigaction(sig, SIG_KTHREAD);
296+
}
297+
298+
static inline void allow_kernel_signal(int sig)
299+
{
300+
/*
301+
* Kernel threads handle their own signals. Let the signal code
302+
* know signals sent by the kernel will be handled, so that they
303+
* don't get silently dropped.
304+
*/
305+
kernel_sigaction(sig, SIG_KTHREAD_KERNEL);
293306
}
294307

295308
static inline void disallow_signal(int sig)

kernel/signal.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ static bool sig_task_ignored(struct task_struct *t, int sig, bool force)
9090
handler == SIG_DFL && !(force && sig_kernel_only(sig)))
9191
return true;
9292

93+
/* Only allow kernel generated signals to this kthread */
94+
if (unlikely((t->flags & PF_KTHREAD) &&
95+
(handler == SIG_KTHREAD_KERNEL) && !force))
96+
return true;
97+
9398
return sig_handler_ignored(handler, sig);
9499
}
95100

0 commit comments

Comments
 (0)