From b16503baa8912a5ec5f599914bfdad898588540f Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 21 Jul 2018 11:35:55 -0500 Subject: signal: send_sig_all no longer needs SEND_SIG_FORCED Now that send_signal always delivers SEND_SIG_PRIV signals to a pid namespace init it is no longer necessary to use SEND_SIG_FORCED when calling do_send_sig_info to ensure that pid namespace inits are signaled and possibly killed. Using SEND_SIG_PRIV is sufficient. So use SEND_SIG_PRIV so that userspace when it receives a SIGTERM can tell that the kernel sent the signal and not some random userspace application. Fixes: b82c32872db2 ("sysrq: use SEND_SIG_FORCED instead of force_sig()") Reviewed-by: Thomas Gleixner Signed-off-by: "Eric W. Biederman" --- drivers/tty/sysrq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c index 06ed20dd01ba..ad1ee5d01b53 100644 --- a/drivers/tty/sysrq.c +++ b/drivers/tty/sysrq.c @@ -348,7 +348,7 @@ static void send_sig_all(int sig) if (is_global_init(p)) continue; - do_send_sig_info(sig, SEND_SIG_FORCED, p, PIDTYPE_MAX); + do_send_sig_info(sig, SEND_SIG_PRIV, p, PIDTYPE_MAX); } read_unlock(&tasklist_lock); } -- cgit v1.2.3 From 961366a01904d460066d65a609c3c2e3051c7903 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 19 Jul 2018 21:31:13 -0500 Subject: signal: Remove the siginfo paramater from kernel_dqueue_signal None of the callers use the it so remove it. Reviewed-by: Thomas Gleixner Signed-off-by: "Eric W. Biederman" --- drivers/usb/gadget/function/f_mass_storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index ca8a4b53c59f..70038a475c9f 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -2311,7 +2311,7 @@ static void handle_exception(struct fsg_common *common) * into a high-priority EXIT exception. */ for (;;) { - int sig = kernel_dequeue_signal(NULL); + int sig = kernel_dequeue_signal(); if (!sig) break; if (sig != SIGUSR1) { -- cgit v1.2.3 From 035150540545f62bada95860ba00fe1e0cd62f63 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Jul 2018 05:31:53 -0500 Subject: signal: Don't send siginfo to kthreads. Today kernel threads never dequeue siginfo so it is pointless to enqueue siginfo for them. The usb gadget mass storage driver goes one farther and uses SEND_SIG_FORCED to guarantee that no siginfo is even enqueued. Generalize the optimization of the usb mass storage driver and never perform an unnecessary allocation when delivering signals to kthreads. Switch the mass storage driver from sending signals with SEND_SIG_FORCED to SEND_SIG_PRIV. As using SEND_SIG_FORCED is now unnecessary. Reviewed-by: Thomas Gleixner Signed-off-by: "Eric W. Biederman" --- drivers/usb/gadget/function/f_mass_storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 70038a475c9f..cb402e7a1e9b 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -403,7 +403,7 @@ static void raise_exception(struct fsg_common *common, enum fsg_state new_state) common->exception_req_tag = common->ep0_req_tag; common->state = new_state; if (common->thread_task) - send_sig_info(SIGUSR1, SEND_SIG_FORCED, + send_sig_info(SIGUSR1, SEND_SIG_PRIV, common->thread_task); } spin_unlock_irqrestore(&common->lock, flags); -- cgit v1.2.3 From 0ab93e9c99f8208c0a1a7b7170c827936268c996 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 13 Sep 2018 11:28:01 +0200 Subject: signal/GenWQE: Fix sending of SIGKILL The genweq_add_file and genwqe_del_file by caching current without using reference counting embed the assumption that a file descriptor will never be passed from one process to another. It even embeds the assumption that the the thread that opened the file will be in existence when the process terminates. Neither of which are guaranteed to be true. Therefore replace caching the task_struct of the opener with pid of the openers thread group id. All the knowledge of the opener is used for is as the target of SIGKILL and a SIGKILL will kill the entire process group. Rename genwqe_force_sig to genwqe_terminate, remove it's unncessary signal argument, update it's ownly caller, and use kill_pid instead of force_sig. The work force_sig does in changing signal handling state is not relevant to SIGKILL sent as SEND_SIG_PRIV. The exact same processess will be killed just with less work, and less confusion. The work done by force_sig is really only needed for handling syncrhonous exceptions. It will still be possible to cause genwqe_device_remove to wait 8 seconds by passing a file descriptor to another process but the possible user after free is fixed. Fixes: eaf4722d4645 ("GenWQE Character device and DDCB queue") Cc: stable@vger.kernel.org Cc: Greg Kroah-Hartman Cc: Frank Haverkamp Cc: Joerg-Stephan Vogt Cc: Michael Jung Cc: Michael Ruettger Cc: Kleber Sacilotto de Souza Cc: Sebastian Ott Cc: Eberhard S. Amann Cc: Gabriel Krisman Bertazi Cc: Guilherme G. Piccoli Signed-off-by: "Eric W. Biederman" --- drivers/misc/genwqe/card_base.h | 2 +- drivers/misc/genwqe/card_dev.c | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/misc/genwqe/card_base.h b/drivers/misc/genwqe/card_base.h index 120738d6e58b..77ed3967c5b0 100644 --- a/drivers/misc/genwqe/card_base.h +++ b/drivers/misc/genwqe/card_base.h @@ -408,7 +408,7 @@ struct genwqe_file { struct file *filp; struct fasync_struct *async_queue; - struct task_struct *owner; + struct pid *opener; struct list_head list; /* entry in list of open files */ spinlock_t map_lock; /* lock for dma_mappings */ diff --git a/drivers/misc/genwqe/card_dev.c b/drivers/misc/genwqe/card_dev.c index f453ab82f0d7..8c1b63a4337b 100644 --- a/drivers/misc/genwqe/card_dev.c +++ b/drivers/misc/genwqe/card_dev.c @@ -52,7 +52,7 @@ static void genwqe_add_file(struct genwqe_dev *cd, struct genwqe_file *cfile) { unsigned long flags; - cfile->owner = current; + cfile->opener = get_pid(task_tgid(current)); spin_lock_irqsave(&cd->file_lock, flags); list_add(&cfile->list, &cd->file_list); spin_unlock_irqrestore(&cd->file_lock, flags); @@ -65,6 +65,7 @@ static int genwqe_del_file(struct genwqe_dev *cd, struct genwqe_file *cfile) spin_lock_irqsave(&cd->file_lock, flags); list_del(&cfile->list); spin_unlock_irqrestore(&cd->file_lock, flags); + put_pid(cfile->opener); return 0; } @@ -275,7 +276,7 @@ static int genwqe_kill_fasync(struct genwqe_dev *cd, int sig) return files; } -static int genwqe_force_sig(struct genwqe_dev *cd, int sig) +static int genwqe_terminate(struct genwqe_dev *cd) { unsigned int files = 0; unsigned long flags; @@ -283,7 +284,7 @@ static int genwqe_force_sig(struct genwqe_dev *cd, int sig) spin_lock_irqsave(&cd->file_lock, flags); list_for_each_entry(cfile, &cd->file_list, list) { - force_sig(sig, cfile->owner); + kill_pid(cfile->opener, SIGKILL, 1); files++; } spin_unlock_irqrestore(&cd->file_lock, flags); @@ -1352,7 +1353,7 @@ static int genwqe_inform_and_stop_processes(struct genwqe_dev *cd) dev_warn(&pci_dev->dev, "[%s] send SIGKILL and wait ...\n", __func__); - rc = genwqe_force_sig(cd, SIGKILL); /* force terminate */ + rc = genwqe_terminate(cd); if (rc) { /* Give kill_timout more seconds to end processes */ for (i = 0; (i < GENWQE_KILL_TIMEOUT) && -- cgit v1.2.3 From a8ebd17160ce364fac6647f223991a8f2f1924b9 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Jul 2018 15:59:17 -0500 Subject: tty_io: Use group_send_sig_info in __do_SACK to note it is a session being killed Replace send_sig and force_sig in __do_SAK with group_send_sig_info the general helper for sending a signal to a process group. This is wordier but it allows specifying PIDTYPE_SID so that the signal code knows the signal went to a session. Both force_sig() and send_sig(..., 1) specify SEND_SIG_PRIV and the new call of group_send_sig_info does that explicitly. This is enough to ensure even a pid namespace init is killed. The global init remains unkillable. The guarantee that __do_SAK tries to provide is a clean path to login to a machine. As the global init is unkillable, if it chooses to hold open a tty it can violate this guarantee. A technique other than killing processes would be needed to provide this guarantee to userspace. The only difference between force_sig and send_sig when sending SIGKILL is that SIGNAL_UNKILLABLE is cleared. This has no affect on the processing of a signal sent with SEND_SIG_PRIV by any process, making it unnecessary, and not behavior that needs to be preserved. force_sig was used originally because it did not take as many locks as send_sig. Today send_sig, force_sig and group_send_sig_info take the same locks when delivering a signal. group_send_sig_info also contains a permission check that force_sig and send_sig do not. However the presence of SEND_SIG_PRIV makes the permission check a noop. So the permission check does not result in any behavioral differences. Signed-off-by: "Eric W. Biederman" --- drivers/tty/tty_io.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 32bc3e3fe4d3..6553247a761f 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -2738,7 +2738,7 @@ void __do_SAK(struct tty_struct *tty) do_each_pid_task(session, PIDTYPE_SID, p) { tty_notice(tty, "SAK: killed process %d (%s): by session\n", task_pid_nr(p), p->comm); - send_sig(SIGKILL, p, 1); + group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID); } while_each_pid_task(session, PIDTYPE_SID, p); /* Now kill any processes that happen to have the tty open */ @@ -2746,7 +2746,7 @@ void __do_SAK(struct tty_struct *tty) if (p->signal->tty == tty) { tty_notice(tty, "SAK: killed process %d (%s): by controlling tty\n", task_pid_nr(p), p->comm); - send_sig(SIGKILL, p, 1); + group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID); continue; } task_lock(p); @@ -2754,7 +2754,7 @@ void __do_SAK(struct tty_struct *tty) if (i != 0) { tty_notice(tty, "SAK: killed process %d (%s): by fd#%d\n", task_pid_nr(p), p->comm, i - 1); - force_sig(SIGKILL, p); + group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID); } task_unlock(p); } while_each_thread(g, p); -- cgit v1.2.3 From ae7795bc6187a15ec51cf258abae656a625f9980 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 25 Sep 2018 11:27:20 +0200 Subject: signal: Distinguish between kernel_siginfo and siginfo Linus recently observed that if we did not worry about the padding member in struct siginfo it is only about 48 bytes, and 48 bytes is much nicer than 128 bytes for allocating on the stack and copying around in the kernel. The obvious thing of only adding the padding when userspace is including siginfo.h won't work as there are sigframe definitions in the kernel that embed struct siginfo. So split siginfo in two; kernel_siginfo and siginfo. Keeping the traditional name for the userspace definition. While the version that is used internally to the kernel and ultimately will not be padded to 128 bytes is called kernel_siginfo. The definition of struct kernel_siginfo I have put in include/signal_types.h A set of buildtime checks has been added to verify the two structures have the same field offsets. To make it easy to verify the change kernel_siginfo retains the same size as siginfo. The reduction in size comes in a following change. Signed-off-by: "Eric W. Biederman" --- drivers/usb/core/devio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 6ce77b33da61..c260ea8808b0 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -582,7 +582,7 @@ static void async_completed(struct urb *urb) { struct async *as = urb->context; struct usb_dev_state *ps = as->ps; - struct siginfo sinfo; + struct kernel_siginfo sinfo; struct pid *pid = NULL; const struct cred *cred = NULL; unsigned long flags; @@ -2599,7 +2599,7 @@ const struct file_operations usbdev_file_operations = { static void usbdev_remove(struct usb_device *udev) { struct usb_dev_state *ps; - struct siginfo sinfo; + struct kernel_siginfo sinfo; while (!list_empty(&udev->filelist)) { ps = list_entry(udev->filelist.next, struct usb_dev_state, list); -- cgit v1.2.3