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.
Tags
None
Referenced Files
F107396556: D30220.diff
Mon, Jan 13, 2:29 PM
Unknown Object (File)
Sun, Jan 5, 4:05 AM
Unknown Object (File)
Fri, Jan 3, 7:48 PM
Unknown Object (File)
Dec 8 2024, 12:30 PM
Unknown Object (File)
Dec 7 2024, 5:54 AM
Unknown Object (File)
Dec 3 2024, 10:15 AM
Unknown Object (File)
Nov 20 2024, 9:30 PM
Unknown Object (File)
Nov 20 2024, 12:43 AM
Subscribers

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
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 39146
Build 36035: arc lint + arc unit

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