2003-03-22 19:36:16

by Dawson Engler

Subject: [CHECKER] race in 2.5.62/kernel/ptrace.c?

Is the following unlocked use of recalc_sigpending a race?

// 2.5.62/kernel/ptrace.c:339:ptrace_notify:
void ptrace_notify(int exit_code)
{
BUG_ON (!(current->ptrace & PT_PTRACED));

/* Let the debugger run. */
current->exit_code = exit_code;
set_current_state(TASK_STOPPED);
notify_parent(current, SIGCHLD);
schedule();

/*
* Signals sent while we were stopped might set TIF_SIGPENDING.
*/
recalc_sigpending();
}


It seems that recalc_sigpending needs to be protected by
&current->sighand->siglock

E.g.,:

2.5.62/kernel/signal.c:1656:sigprocmask:
recalc_sigpending();
spin_unlock_irq(&current->sighand->siglock);

2.5.62/kernel/signal.c:2115:sys_sigprocmas

spin_lock_irq(&current->sighand->siglock);
old_set = current->blocked.sig[0];

...

recalc_sigpending();
spin_unlock_irq(&current->sighand->siglock);


Or does it not need a lock? (Or am I missing the lock?)

2003-03-22 21:35:42

by Andrew Morton

Subject: Re: [CHECKER] race in 2.5.62/kernel/ptrace.c?

Dawson Engler <[email protected]> wrote:
>
> Is the following unlocked use of recalc_sigpending a race?
>
> // 2.5.62/kernel/ptrace.c:339:ptrace_notify:
> void ptrace_notify(int exit_code)
> {
> BUG_ON (!(current->ptrace & PT_PTRACED));
>
> /* Let the debugger run. */
> current->exit_code = exit_code;
> set_current_state(TASK_STOPPED);
> notify_parent(current, SIGCHLD);
> schedule();
>
> /*
> * Signals sent while we were stopped might set TIF_SIGPENDING.
> */
> recalc_sigpending();
> }
>

I think so. To find out I shall send a patch to Linus and see if
I get shouted at.