5 files changed, 66 insertions, 46 deletions

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index b361d28061d0..7079ddaf0731 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -595,7 +595,7 @@ static unsigned long ptrace_get_dr7(struct perf_event *bp[])
         return dr7;
 }
 
-static struct perf_event *
+static int
 ptrace_modify_breakpoint(struct perf_event *bp, int len, int type,
                          struct task_struct *tsk, int disabled)
 {
@@ -609,11 +609,11 @@ ptrace_modify_breakpoint(struct perf_event *bp, int len, int type,
          * written the address register first
          */
         if (!bp)
-                return ERR_PTR(-EINVAL);
+                return -EINVAL;
 
         err = arch_bp_generic_fields(len, type, &gen_len, &gen_type);
         if (err)
-                return ERR_PTR(err);
+                return err;
 
         attr = bp->attr;
         attr.bp_len = gen_len;
@@ -658,28 +658,17 @@ restore:
                                 if (!second_pass)
                                         continue;
 
-                                thread->ptrace_bps[i] = NULL;
-                                bp = ptrace_modify_breakpoint(bp, len, type,
+                                rc = ptrace_modify_breakpoint(bp, len, type,
                                                               tsk, 1);
-                                if (IS_ERR(bp)) {
-                                        rc = PTR_ERR(bp);
-                                        thread->ptrace_bps[i] = NULL;
+                                if (rc)
                                         break;
-                                }
-                                thread->ptrace_bps[i] = bp;
                         }
                         continue;
                 }
 
-                bp = ptrace_modify_breakpoint(bp, len, type, tsk, 0);
-
-                /* Incorrect bp, or we have a bug in bp API */
-                if (IS_ERR(bp)) {
-                        rc = PTR_ERR(bp);
-                        thread->ptrace_bps[i] = NULL;
+                rc = ptrace_modify_breakpoint(bp, len, type, tsk, 0);
+                if (rc)
                         break;
-                }
-                thread->ptrace_bps[i] = bp;
         }
         /*
          * Make a second pass to free the remaining unused breakpoints
@@ -737,26 +726,32 @@ static int ptrace_set_breakpoint_addr(struct task_struct *tsk, int nr,
                 attr.disabled = 1;
 
                 bp = register_user_hw_breakpoint(&attr, ptrace_triggered, tsk);
+
+                /*
+                 * CHECKME: the previous code returned -EIO if the addr wasn't
+                 * a valid task virtual addr. The new one will return -EINVAL in
+                 *  this case.
+                 * -EINVAL may be what we want for in-kernel breakpoints users,
+                 * but -EIO looks better for ptrace, since we refuse a register
+                 * writing for the user. And anyway this is the previous
+                 * behaviour.
+                 */
+                if (IS_ERR(bp))
+                        return PTR_ERR(bp);
+
+                t->ptrace_bps[nr] = bp;
         } else {
+                int err;
+
                 bp = t->ptrace_bps[nr];
-                t->ptrace_bps[nr] = NULL;
 
                 attr = bp->attr;
                 attr.bp_addr = addr;
-                bp = modify_user_hw_breakpoint(bp, &attr);
+                err = modify_user_hw_breakpoint(bp, &attr);
+                if (err)
+                        return err;
         }
-        /*
-         * CHECKME: the previous code returned -EIO if the addr wasn't a
-         * valid task virtual addr. The new one will return -EINVAL in this
-         * case.
-         * -EINVAL may be what we want for in-kernel breakpoints users, but
-         * -EIO looks better for ptrace, since we refuse a register writing
-         * for the user. And anyway this is the previous behaviour.
-         */
-        if (IS_ERR(bp))
-                return PTR_ERR(bp);
 
-        t->ptrace_bps[nr] = bp;
 
         return 0;
 }
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index 42da1ce19ec0..69f07a9f1277 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -55,7 +55,7 @@ register_user_hw_breakpoint(struct perf_event_attr *attr,
                             struct task_struct *tsk);
 
 /* FIXME: only change from the attr, and don't unregister */
-extern struct perf_event *
+extern int
 modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr);
 
 /*
@@ -91,7 +91,7 @@ static inline struct perf_event *
 register_user_hw_breakpoint(struct perf_event_attr *attr,
                             perf_overflow_handler_t triggered,
                             struct task_struct *tsk)    { return NULL; }
-static inline struct perf_event *
+static inline int
 modify_user_hw_breakpoint(struct perf_event *bp,
                           struct perf_event_attr *attr) { return NULL; }
 static inline struct perf_event *
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index bf3329413e18..64a53f74c9a9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -872,6 +872,8 @@ extern void perf_output_copy(struct perf_output_handle *handle,
                              const void *buf, unsigned int len);
 extern int perf_swevent_get_recursion_context(void);
 extern void perf_swevent_put_recursion_context(int rctx);
+extern void perf_event_enable(struct perf_event *event);
+extern void perf_event_disable(struct perf_event *event);
 #else
 static inline void
 perf_event_task_sched_in(struct task_struct *task, int cpu)             { }
@@ -902,7 +904,8 @@ static inline void perf_event_fork(struct task_struct *tsk)		{ }
 static inline void perf_event_init(void)                                { }
 static inline int  perf_swevent_get_recursion_context(void)  { return -1; }
 static inline void perf_swevent_put_recursion_context(int rctx)         { }
-
+static inline void perf_event_enable(struct perf_event *event)          { }
+static inline void perf_event_disable(struct perf_event *event)         { }
 #endif
 
 #define perf_output_put(handle, x) \
diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index 03a0773ac2b2..366eedf949c0 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -320,18 +320,40 @@ EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
  * @triggered: callback to trigger when we hit the breakpoint
  * @tsk: pointer to 'task_struct' of the process to which the address belongs
  */
-struct perf_event *
-modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
+int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
 {
-        /*
-         * FIXME: do it without unregistering
-         * - We don't want to lose our slot
-         * - If the new bp is incorrect, don't lose the older one
-         */
-        unregister_hw_breakpoint(bp);
+        u64 old_addr = bp->attr.bp_addr;
+        int old_type = bp->attr.bp_type;
+        int old_len = bp->attr.bp_len;
+        int err = 0;
+
+        perf_event_disable(bp);
+
+        bp->attr.bp_addr = attr->bp_addr;
+        bp->attr.bp_type = attr->bp_type;
+        bp->attr.bp_len = attr->bp_len;
+
+        if (attr->disabled)
+                goto end;
 
-        return perf_event_create_kernel_counter(attr, -1, bp->ctx->task->pid,
-                                                bp->overflow_handler);
+        err = arch_validate_hwbkpt_settings(bp, bp->ctx->task);
+        if (!err)
+                perf_event_enable(bp);
+
+        if (err) {
+                bp->attr.bp_addr = old_addr;
+                bp->attr.bp_type = old_type;
+                bp->attr.bp_len = old_len;
+                if (!bp->attr.disabled)
+                        perf_event_enable(bp);
+
+                return err;
+        }
+
+end:
+        bp->attr.disabled = attr->disabled;
+
+        return 0;
 }
 EXPORT_SYMBOL_GPL(modify_user_hw_breakpoint);
 
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index fd43ff4ac860..3b0cf86eee84 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -567,7 +567,7 @@ static void __perf_event_disable(void *info)
  * is the current context on this CPU and preemption is disabled,
  * hence we can't get into perf_event_task_sched_out for this context.
  */
-static void perf_event_disable(struct perf_event *event)
+void perf_event_disable(struct perf_event *event)
 {
         struct perf_event_context *ctx = event->ctx;
         struct task_struct *task = ctx->task;
@@ -971,7 +971,7 @@ static void __perf_event_enable(void *info)
  * perf_event_for_each_child or perf_event_for_each as described
  * for perf_event_disable.
  */
-static void perf_event_enable(struct perf_event *event)
+void perf_event_enable(struct perf_event *event)
 {
         struct perf_event_context *ctx = event->ctx;
         struct task_struct *task = ctx->task;