Not sure you will like it, hence the separate patch.

ptrace_{get,set}siginfo() need ACCESS_ONCE() to fix the theoretical
problem, and they also need a comment to explain why it is safe to
dereference ->siginfo under ->siglock.

I think it would be more readable if we add the trivial helper, just
to avoid duplicating the comment.

---

 kernel/ptrace.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

--- PU/kernel/ptrace.c~58_CONTEXT_SIGINFO       2009-09-24 02:14:17.000000000 
+0200
+++ PU/kernel/ptrace.c  2009-09-24 02:26:26.000000000 +0200
@@ -326,6 +326,17 @@ static u32 ptrace_report_exec(enum utrac
        return UTRACE_STOP;
 }
 
+static inline siginfo_t *context_siginfo(struct ptrace_context *context)
+{
+       /*
+        * Make sure the compiler reads ->siginfo only once, if we race
+        * with SIGKILL ->siginfo can be cleared under us. But since we
+        * hold ->siglock the memory it points to can't go away, see the
+        * comment in ptrace_report_signal() below.
+        */
+       return ACCESS_ONCE(context->siginfo);
+}
+
 static void ptrace_resume_signal(struct utrace_engine *engine,
                                        struct task_struct *tracee, long data)
 {
@@ -334,13 +345,8 @@ static void ptrace_resume_signal(struct 
 
        if (!lock_task_sighand(tracee, &flags))
                return;
-       /*
-        * Make sure the compiler reads ->siginfo only once, if we race
-        * with SIGKILL ->siginfo can be cleared under us. But since we
-        * hold ->siglock the memory it points to can't go away, see the
-        * comment in ptrace_report_signal() below.
-        */
-       info = ACCESS_ONCE(ptrace_context(engine)->siginfo);
+
+       info = context_siginfo(ptrace_context(engine));
        WARN_ON(!info && !(tracee->signal->flags & SIGNAL_GROUP_EXIT));
 
        if (likely(info) && info->si_signo != data) {

Reply via email to