Skip to content

ptrace: fix race between ptrace_resume() and wait_task_stopped()

Oleg Nesterov lkml 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest2 patches 0 repliesNew

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2ptrace: fix race between ptrace_resume() and wait_task_stopped()kernel/ptrace.c+20 -00 replies
2/2ptrace: ptrace_detach() can no longer race with SIGKILLkernel/ptrace.c+9 -100 replies

Reviews trailers per patch

None of the 2 patches has a Reviewed-by or Acked-by.

PatchPavel Labath
1/2T-b
2/2

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v1 thread

Cover letter [PATCH 0/2] ptrace: fix race between ptrace_resume() and wait_task_stopped()
Quoted 3 lines · Pavel Labath

I have tested your patch and I can confirm that the error is gone when running a patched kernel.

Great! Thanks a lot for the detailed/clear report and testing.

So I am sending this fix + another patch. 2/2 is "while at it" change, just because ptrace_detach() can resume the tracee with the new code too, so it makes sense to add a comment and remove the outdated logic.

Quoted 3 lines

I am still seeing one very rare failure where the SIGUSR does not appear to be reported. However, I will need to dig around this a bit more to make sure there is no error on our end.

Hmm, perhaps we have (yet) another bug... please let me know if/when you have more details.

Quoted 5 lines

Now I am thinking about how to work around these bugs, as our code will need to run on unpatched kernels as well. As for this ptrace/waitpid race, I think I will just refactor the code to make wait and ptrace calls on the same thread. This should sidestep the race, right?

Yes sure, this will hide the problem.

Quoted 5 lines

Regarding your bug, I am not exactly sure what are the implications. Could you briefly describe the situations in which this behavior can occur? Am I correct in understanding that this is always a race between a SIGKILL and another non-lethal signal? And that the SIGKILL will be (eventually) reported?

No, SIGKILL can be never reported. But note that ptrace_stop() does

	set_current_state(TASK_TRACED);

	current->last_siginfo = info;
	current->exit_code = exit_code;

and this is another case when wait_task_stopped() can consume/report this exit_code even if the tracee won't actually stop because it is killed.

Usually this is not that bad, we can pretend that it was killed after stop. Still this can confuse the debugger which sends SIGKILL to the stopped tracee. We need more fatal_signal_pending() checks in ptrace_stop(). And in fact in get_signal(), I think. The problem is that we need other cleanups here. But fortunately this problem is minor.

Oleg.

Read the cover letter in its thread