Page MenuHomeFreeBSD

ktrace: Fix a race with fork()
ClosedPublic

Authored by markj on May 26 2021, 6:14 PM.

Details

Summary

ktrace(2) may toggle trace points in any of

  1. a single process
  2. all members of a process group
  3. all descendents of the processes in 1 or 2

In the first two cases, we do not permit the operation if the process is
new or not visible. However, in case 3 we did not enforce this
restriction for descendents. As a result, the assertions about the
child in ktrprocfork() may be violated.

Move these checks into ktrops() so that they are applied consistently.

Allow KTROP_CLEAR for nascent processes. Otherwise, there is a window
where we cannot clear trace points for a nascent child if they are
inherited from the parent.

Fix style in ktrsetchildren().

Reported by: syzbot+d96676592978f137e05c@syzkaller.appspotmail.com
Reported by: syzbot+7c98fcf84a4439f2817f@syzkaller.appspotmail.com

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib added inline comments.
sys/kern/kern_ktrace.c
1258

This is arguably wrong, as well as the fact that all children are walked. But at least it is consistent with the fact that orphans are not iterated.

For p_children, the fix would probably to check that p->p_pptr == proc_realparent(p).

This revision is now accepted and ready to land.May 27 2021, 12:13 AM
sys/kern/kern_ktrace.c
1258

I don't quite follow "orphans are not iterated". Orphans are added to the reaper's p_children list as soon as the parent dies, so we visit them too, I believe. Skipping processes for which p->p_pptr != proc_realparent(p) seems reasonable: it would exclude debuggees and orphans whose parent has not yet been reaped.

sys/kern/kern_ktrace.c
1258

When we iterate to add all current descendants of the process to the traced list (KTR_SET and KTRFLAG_DESCEND), we should iterate both p_children (but avoid ptraced reparented child which is debuggee) and p_orphans, since orphans are our children being ptraced by other process.

Similarly, when we clear ktracing for descendants, we should clear it for siblings and orphans now, not delaying it for the debugger termination.

This revision was automatically updated to reflect the committed changes.