Page MenuHomeFreeBSD

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

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

Details

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
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

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.

A comment should be added from your last note.

This revision is now accepted and ready to land.May 12 2021, 7:00 PM
This revision now requires review to proceed.May 12 2021, 8:59 PM
This revision is now accepted and ready to land.May 12 2021, 10:28 PM