livepatch: Remove signal sysfs attribute
authorMiroslav Benes <mbenes@suse.cz>
Tue, 15 Jan 2019 16:45:07 +0000 (17:45 +0100)
committerJiri Kosina <jkosina@suse.cz>
Wed, 16 Jan 2019 21:09:33 +0000 (22:09 +0100)
The fake signal is send automatically now. We can rely on it completely
and remove the sysfs attribute.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Documentation/ABI/testing/sysfs-kernel-livepatch
Documentation/livepatch/livepatch.txt
kernel/livepatch/core.c
kernel/livepatch/transition.c
kernel/livepatch/transition.h

index dac7e1e..85db352 100644 (file)
@@ -33,18 +33,6 @@ Description:
                An attribute which indicates whether the patch is currently in
                transition.
 
-What:          /sys/kernel/livepatch/<patch>/signal
-Date:          Nov 2017
-KernelVersion: 4.15.0
-Contact:       live-patching@vger.kernel.org
-Description:
-               A writable attribute that allows administrator to affect the
-               course of an existing transition. Writing 1 sends a fake
-               signal to all remaining blocking tasks. The fake signal
-               means that no proper signal is delivered (there is no data in
-               signal pending structures). Tasks are interrupted or woken up,
-               and forced to change their patched state.
-
 What:          /sys/kernel/livepatch/<patch>/force
 Date:          Nov 2017
 KernelVersion: 4.15.0
index 407e0f0..4627b41 100644 (file)
@@ -158,13 +158,11 @@ If a patch is in transition, this file shows 0 to indicate the task is
 unpatched and 1 to indicate it's patched.  Otherwise, if no patch is in
 transition, it shows -1.  Any tasks which are blocking the transition
 can be signaled with SIGSTOP and SIGCONT to force them to change their
-patched state. This may be harmful to the system though.
-/sys/kernel/livepatch/<patch>/signal attribute provides a better alternative.
-Writing 1 to the attribute sends a fake signal to all remaining blocking
-tasks. No proper signal is actually delivered (there is no data in signal
-pending structures). Tasks are interrupted or woken up, and forced to change
-their patched state. Despite the sysfs attribute the fake signal is also sent
-every 15 seconds automatically.
+patched state. This may be harmful to the system though. Sending a fake signal
+to all remaining blocking tasks is a better alternative. No proper signal is
+actually delivered (there is no data in signal pending structures). Tasks are
+interrupted or woken up, and forced to change their patched state. The fake
+signal is automatically sent every 15 seconds.
 
 Administrator can also affect a transition through
 /sys/kernel/livepatch/<patch>/force attribute. Writing 1 there clears
@@ -412,8 +410,8 @@ Information about the registered patches can be found under
 /sys/kernel/livepatch. The patches could be enabled and disabled
 by writing there.
 
-/sys/kernel/livepatch/<patch>/signal and /sys/kernel/livepatch/<patch>/force
-attributes allow administrator to affect a patching operation.
+/sys/kernel/livepatch/<patch>/force attributes allow administrator to affect a
+patching operation.
 
 See Documentation/ABI/testing/sysfs-kernel-livepatch for more details.
 
index adca5cf..fe19933 100644 (file)
@@ -313,7 +313,6 @@ static int klp_write_object_relocations(struct module *pmod,
  * /sys/kernel/livepatch/<patch>
  * /sys/kernel/livepatch/<patch>/enabled
  * /sys/kernel/livepatch/<patch>/transition
- * /sys/kernel/livepatch/<patch>/signal
  * /sys/kernel/livepatch/<patch>/force
  * /sys/kernel/livepatch/<patch>/<object>
  * /sys/kernel/livepatch/<patch>/<object>/<function,sympos>
@@ -382,35 +381,6 @@ static ssize_t transition_show(struct kobject *kobj,
                        patch == klp_transition_patch);
 }
 
-static ssize_t signal_store(struct kobject *kobj, struct kobj_attribute *attr,
-                           const char *buf, size_t count)
-{
-       struct klp_patch *patch;
-       int ret;
-       bool val;
-
-       ret = kstrtobool(buf, &val);
-       if (ret)
-               return ret;
-
-       if (!val)
-               return count;
-
-       mutex_lock(&klp_mutex);
-
-       patch = container_of(kobj, struct klp_patch, kobj);
-       if (patch != klp_transition_patch) {
-               mutex_unlock(&klp_mutex);
-               return -EINVAL;
-       }
-
-       klp_send_signals();
-
-       mutex_unlock(&klp_mutex);
-
-       return count;
-}
-
 static ssize_t force_store(struct kobject *kobj, struct kobj_attribute *attr,
                           const char *buf, size_t count)
 {
@@ -442,12 +412,10 @@ static ssize_t force_store(struct kobject *kobj, struct kobj_attribute *attr,
 
 static struct kobj_attribute enabled_kobj_attr = __ATTR_RW(enabled);
 static struct kobj_attribute transition_kobj_attr = __ATTR_RO(transition);
-static struct kobj_attribute signal_kobj_attr = __ATTR_WO(signal);
 static struct kobj_attribute force_kobj_attr = __ATTR_WO(force);
 static struct attribute *klp_patch_attrs[] = {
        &enabled_kobj_attr.attr,
        &transition_kobj_attr.attr,
-       &signal_kobj_attr.attr,
        &force_kobj_attr.attr,
        NULL
 };
index ea7697b..183b208 100644 (file)
@@ -348,6 +348,47 @@ done:
 }
 
 /*
+ * Sends a fake signal to all non-kthread tasks with TIF_PATCH_PENDING set.
+ * Kthreads with TIF_PATCH_PENDING set are woken up.
+ */
+static void klp_send_signals(void)
+{
+       struct task_struct *g, *task;
+
+       if (klp_signals_cnt == SIGNALS_TIMEOUT)
+               pr_notice("signaling remaining tasks\n");
+
+       read_lock(&tasklist_lock);
+       for_each_process_thread(g, task) {
+               if (!klp_patch_pending(task))
+                       continue;
+
+               /*
+                * There is a small race here. We could see TIF_PATCH_PENDING
+                * set and decide to wake up a kthread or send a fake signal.
+                * Meanwhile the task could migrate itself and the action
+                * would be meaningless. It is not serious though.
+                */
+               if (task->flags & PF_KTHREAD) {
+                       /*
+                        * Wake up a kthread which sleeps interruptedly and
+                        * still has not been migrated.
+                        */
+                       wake_up_state(task, TASK_INTERRUPTIBLE);
+               } else {
+                       /*
+                        * Send fake signal to all non-kthread tasks which are
+                        * still not migrated.
+                        */
+                       spin_lock_irq(&task->sighand->siglock);
+                       signal_wake_up(task, 0);
+                       spin_unlock_irq(&task->sighand->siglock);
+               }
+       }
+       read_unlock(&tasklist_lock);
+}
+
+/*
  * Try to switch all remaining tasks to the target patch state by walking the
  * stacks of sleeping tasks and looking for any to-be-patched or
  * to-be-unpatched functions.  If such functions are found, the task can't be
@@ -587,47 +628,6 @@ void klp_copy_process(struct task_struct *child)
 }
 
 /*
- * Sends a fake signal to all non-kthread tasks with TIF_PATCH_PENDING set.
- * Kthreads with TIF_PATCH_PENDING set are woken up.
- */
-void klp_send_signals(void)
-{
-       struct task_struct *g, *task;
-
-       if (klp_signals_cnt == SIGNALS_TIMEOUT)
-               pr_notice("signaling remaining tasks\n");
-
-       read_lock(&tasklist_lock);
-       for_each_process_thread(g, task) {
-               if (!klp_patch_pending(task))
-                       continue;
-
-               /*
-                * There is a small race here. We could see TIF_PATCH_PENDING
-                * set and decide to wake up a kthread or send a fake signal.
-                * Meanwhile the task could migrate itself and the action
-                * would be meaningless. It is not serious though.
-                */
-               if (task->flags & PF_KTHREAD) {
-                       /*
-                        * Wake up a kthread which sleeps interruptedly and
-                        * still has not been migrated.
-                        */
-                       wake_up_state(task, TASK_INTERRUPTIBLE);
-               } else {
-                       /*
-                        * Send fake signal to all non-kthread tasks which are
-                        * still not migrated.
-                        */
-                       spin_lock_irq(&task->sighand->siglock);
-                       signal_wake_up(task, 0);
-                       spin_unlock_irq(&task->sighand->siglock);
-               }
-       }
-       read_unlock(&tasklist_lock);
-}
-
-/*
  * Drop TIF_PATCH_PENDING of all tasks on admin's request. This forces an
  * existing transition to finish.
  *
index f9d0bc0..322db16 100644 (file)
@@ -11,7 +11,6 @@ void klp_cancel_transition(void);
 void klp_start_transition(void);
 void klp_try_complete_transition(void);
 void klp_reverse_transition(void);
-void klp_send_signals(void);
 void klp_force_transition(void);
 
 #endif /* _LIVEPATCH_TRANSITION_H */