fork: Suspend other threads if both RFPROC and RFMEM are not set
ClosedPublic
Actions

Authored by markj on May 12 2021, 12:25 AM.

Details

Reviewers

Commits

rGcf60931c32d7: fork: Suspend other threads if both RFPROC and RFMEM are not set
rG9246b3090cbc: fork: Suspend other threads if both RFPROC and RFMEM are not set

Summary

Otherwise, a multithreaded parent process may trigger races in
vm_forkproc() if one thread calls rfork() with RFMEM set and another
calls rfork() without RFMEM.

Also simplify vm_forkproc() a bit, vmspace_unshare() already checks to
see if the address space is shared.

Reported by: syzbot+0aa7c2bec74c4066c36f@syzkaller.appspotmail.com
Reported by: syzbot+ea84cb06937afeae609d@syzkaller.appspotmail.com

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 39146
Build 36035: arc lint + arc unit

Event Timeline

markj created this revision.May 12 2021, 12:25 AM

Herald added a subscriber: imp. · View Herald TranscriptMay 12 2021, 12:25 AM

markj requested review of this revision.May 12 2021, 12:25 AM

Harbormaster completed remote builds in B39130: Diff 89053.May 12 2021, 12:25 AM

markj added a reviewer: kib.May 12 2021, 12:25 AM

Could you please show specifics of things going wrong?

My concern is that if other thread might cause troubles for vmspace_unshare(), then other process sharing the address space, can do that as well, and single-threading does not stop other processes.

In D30220#678711, @kib wrote:

Could you please show specifics of things going wrong?

Here are the reports:

https://syzkaller.appspot.com/bug?id=879bebd49d9fb81ae0ac75307da44a9f3d979f7d
https://syzkaller.appspot.com/bug?id=a39b5a3f1bed634a4c8f2cd76d571917d7e20a96

My concern is that if other thread might cause troubles for vmspace_unshare(), then other process sharing the address space, can do that as well, and single-threading does not stop other processes.

Consider a scenario where two threads concurrently call rfork(RFMEM | RFPROC) and rfork(0). The former will create a new process sharing the vmspace of the parent, and because the refcount test in vm_forkproc() is racy, the latter may fail to call vmspace_unshare() when it should. My reasoning for the diff is that it ensures that the refcount will not transition 1->2 while other threads are suspended.