To simplify the review:
static int ptrace_rw_siginfo(struct task_struct *tracee,
struct ptrace_context *context,
siginfo_t *info, bool write)
{
unsigned long flags;
int err;
switch (get_stop_event(context)) {
case 0: /* jctl stop */
return -EINVAL;
case PTRACE_EVENT_SIGNAL:
err = -ESRCH;
if (lock_task_sighand(tracee, &flags)) {
if (likely(task_is_traced(tracee))) {
if (write)
*context->siginfo = *info;
else
*info = *context->siginfo;
err = 0;
}
unlock_task_sighand(tracee, &flags);
}
return err;
default:
if (!write) {
memset(info, 0, sizeof(*info));
info->si_signo = SIGTRAP;
info->si_code = context->stop_code &
PTRACE_EVENT_MASK;
info->si_pid = task_pid_vnr(tracee);
info->si_uid = task_uid(tracee);
}
return 0;
}
}
ptrace_request() uses this helper directly.
The logic is very simple, we never check ->siginfo != NULL.
If PTRACE_EVENT_SIGNAL, then ->siginfo must be valid. To protect
against the race with SIGKILL we simply check task_is_traced()
under ->siglock.
With this change we don't care about how/when ->siginfo is changed.
ptrace_rw_siginfo() could read ->siginfo lockeless, but this will
complicate the code. Let's do this later if needed, the curent code
takes ->siglock too.
---
kernel/ptrace.c | 93 ++++++++++++++++++++------------------------------------
1 file changed, 34 insertions(+), 59 deletions(-)
--- PU/kernel/ptrace.c~73_SIGINFO_TWEAKS 2009-10-09 14:34:38.000000000
+0200
+++ PU/kernel/ptrace.c 2009-10-09 16:00:11.000000000 +0200
@@ -786,65 +786,40 @@ static int ptrace_rw_siginfo(struct task
siginfo_t *info, bool write)
{
unsigned long flags;
- siginfo_t *context_info;
- int err = -ESRCH;
-
- if (!lock_task_sighand(tracee, &flags))
- return err;
- /*
- * Make sure the compiler reads ->siginfo only once, if we race
- * with SIGKILL ->siginfo can be cleared under us. But the memory
- * it points to can't go away, and since we hold ->siglock we can't
- * race with get_signal_to_deliver() pathes clobber this memory.
- * See also the comment in ptrace_report_signal().
- */
- context_info = ACCESS_ONCE(context->siginfo);
- if (context_info) {
- if (write)
- *context_info = *info;
- else
- *info = *context_info;
- err = 0;
- }
- unlock_task_sighand(tracee, &flags);
-
- return err;
-}
+ int err;
-static int ptrace_getsiginfo(struct ptrace_context *context,
- struct task_struct *tracee, siginfo_t *info)
-{
- /* jctl stop ? */
- if (!ptrace_event_pending(context))
+ switch (get_stop_event(context)) {
+ case 0: /* jctl stop */
return -EINVAL;
- if (context->siginfo)
- return ptrace_rw_siginfo(tracee, context, info, false);
-
- memset(info, 0, sizeof(*info));
- info->si_signo = SIGTRAP;
- info->si_code = context->stop_code & PTRACE_EVENT_MASK;
- info->si_pid = task_pid_vnr(tracee);
- info->si_uid = task_uid(tracee);
-
- return 0;
-}
+ case PTRACE_EVENT_SIGNAL:
+ err = -ESRCH;
+ if (lock_task_sighand(tracee, &flags)) {
+ if (likely(task_is_traced(tracee))) {
+ if (write)
+ *context->siginfo = *info;
+ else
+ *info = *context->siginfo;
+ err = 0;
+ }
+ unlock_task_sighand(tracee, &flags);
+ }
-static int ptrace_setsiginfo(struct ptrace_context *context,
- struct task_struct *tracee, siginfo_t *info)
-{
- /* jctl stop ? */
- if (!ptrace_event_pending(context))
- return -EINVAL;
+ return err;
- if (context->siginfo)
- return ptrace_rw_siginfo(tracee, context, info, true);
+ default:
+ if (!write) {
+ memset(info, 0, sizeof(*info));
+ info->si_signo = SIGTRAP;
+ info->si_code = context->stop_code & PTRACE_EVENT_MASK;
+ info->si_pid = task_pid_vnr(tracee);
+ info->si_uid = task_uid(tracee);
+ }
- /* compatibility: pretend it was updated */
- return 0;
+ return 0;
+ }
}
-
#ifdef PTRACE_SINGLESTEP
#define is_singlestep(request) ((request) == PTRACE_SINGLESTEP)
#else
@@ -1047,8 +1022,8 @@ int ptrace_request(struct task_struct *c
break;
case PTRACE_GETSIGINFO:
- ret = ptrace_getsiginfo(ptrace_context(engine),
- child, &siginfo);
+ ret = ptrace_rw_siginfo(child, ptrace_context(engine),
+ &siginfo, false);
if (!ret)
ret = copy_siginfo_to_user((siginfo_t __user *) data,
&siginfo);
@@ -1059,8 +1034,8 @@ int ptrace_request(struct task_struct *c
sizeof siginfo))
ret = -EFAULT;
else
- ret = ptrace_setsiginfo(ptrace_context(engine),
- child, &siginfo);
+ ret = ptrace_rw_siginfo(child, ptrace_context(engine),
+ &siginfo, true);
break;
case PTRACE_DETACH: /* detach a process that was attached. */
@@ -1218,8 +1193,8 @@ int compat_ptrace_request(struct task_st
break;
case PTRACE_GETSIGINFO:
- ret = ptrace_getsiginfo(ptrace_context(engine),
- child, &siginfo);
+ ret = ptrace_rw_siginfo(child, ptrace_context(engine),
+ &siginfo, false);
if (!ret)
ret = copy_siginfo_to_user32(
(struct compat_siginfo __user *) datap,
@@ -1232,8 +1207,8 @@ int compat_ptrace_request(struct task_st
&siginfo, (struct compat_siginfo __user *) datap))
ret = -EFAULT;
else
- ret = ptrace_setsiginfo(ptrace_context(engine),
- child, &siginfo);
+ ret = ptrace_rw_siginfo(child, ptrace_context(engine),
+ &siginfo, true);
break;
default:
