Re: Sending signals to a kernel thread, broken in 2.6.22

[Oleg Nesterov <oleg@tv-sign.ru>]

On 05/30, Oleg Nesterov wrote:
>
> Alan Stern wrote:
> >
> > The g_file_storage driver uses a kernel thread and communicates with
> > that thread in part by means of signals.  It also relies on the thread
> > receiving signals from userspace as an indication that the thread
> > should terminate.
> >
> > This was all working in 2.6.21, but as of 2.6.22-rc3 the signal
> > delivery mechanism (entirely within the kernel!) is no longer
> > functional.
> 
> I guess you mean drivers/usb/gadget/file_storage.c
> 
> 	fsg_main_thread:
> 		
> 		siginitsetinv(&fsg->thread_signal_mask, SIGTERM | ...);
> 		sigprocmask(SIG_SETMASK, &fsg->thread_signal_mask, NULL);
> 
> Yes?
> 
> Please look at
> 
> 	change kernel threads to ignore signals instead of blocking them
> 	commit: 10ab825bdef8df510f99c703a5a2d9b13a4e31a5
> 
> I think you can convert the code above to use allow_signal().

something like untested/uncompiled patch below, what do you think?

Oleg.

--- u/drivers/usb/gadget/file_storage.c~	2007-05-30 16:24:19.000000000 +0400
+++ u/drivers/usb/gadget/file_storage.c	2007-05-30 16:29:21.000000000 +0400
@@ -686,7 +686,6 @@ struct fsg_dev {
 	int			thread_wakeup_needed;
 	struct completion	thread_notifier;
 	struct task_struct	*thread_task;
-	sigset_t		thread_signal_mask;
 
 	int			cmnd_size;
 	u8			cmnd[MAX_COMMAND_SIZE];
@@ -3277,8 +3276,7 @@ static void handle_exception(struct fsg_
 	/* Clear the existing signals.  Anything but SIGUSR1 is converted
 	 * into a high-priority EXIT exception. */
 	for (;;) {
-		sig = dequeue_signal_lock(current, &fsg->thread_signal_mask,
-				&info);
+		sig = dequeue_signal_lock(current, &current->blocked, &info);
 		if (!sig)
 			break;
 		if (sig != SIGUSR1) {
@@ -3431,10 +3429,10 @@ static int fsg_main_thread(void *fsg_)
 
 	/* Allow the thread to be killed by a signal, but set the signal mask
 	 * to block everything but INT, TERM, KILL, and USR1. */
-	siginitsetinv(&fsg->thread_signal_mask, sigmask(SIGINT) |
-			sigmask(SIGTERM) | sigmask(SIGKILL) |
-			sigmask(SIGUSR1));
-	sigprocmask(SIG_SETMASK, &fsg->thread_signal_mask, NULL);
+	allow_signal(SIGINT);
+	allow_signal(SIGTERM);
+	allow_signal(SIGKILL);
+	allow_signal(SIGUSR1);
 
 	/* Arrange for userspace references to be interpreted as kernel
 	 * pointers.  That way we can pass a kernel pointer to a routine