On Sun, Apr 10, 2016 at 12:25:55AM +0200, Mark Kettenis wrote: > > I really hope people won't deliberately write code that allows for > simultanious execution of pledge(2) in multiple threads. In fact the > only justification for calling pledge(2) in a multi-threaded process > is if you wanted to create threads and then call pledge(2) to prevent > the creation of more threads. > > So the purpose of this diff is to prevent an attacker from exploiting > the race between multiple pledge(2) calls to circumvent the pledge? > Currently that risk isn't really there. Or at least it could be > avoided by slightly reorganizing the code. But I guess things get > more complicated once the whitelist stuff gets activated. > > I agree with Ted's remark about using rwlock(9) though. >
First, thanks for your comments. Yes, whitelist stuff is the reason for this diff, even if theorically there is already a race in pledge(2) due to malloc(M_WAITOK), which could sleep, so a thread inside sys_pledge() could release KERN_LOCK, letting another thread of same process enter also in sys_pledge(). I am trying to move the current complexity of the algo for checking whitelisted paths from namei() to sys_pledge(), in order to reduce the performance impact at namei() time (and the possible use of pledge(2) for attacking locally the system). As result, sys_pledge() would have more work to do, and to avoid doing all his stuff in one time under KERN_LOCK (and becoming a vector attack for DoSing locally), it could have to yield() in the middle of the processing (or completely release KERN_LOCK for processing and grab it before commiting ? I will think about that too). It is mainly parsing the list of paths and do preprocessing on it in order to organize them in a way namei() could be O(log n) concerning whitelisted paths validation. About the use of pledge(2) in multithread programs, I agree that pledge(2) isn't intented to be used in this way. But this syscall has process-wide effect and it should behave consistently if called from multithreaded programs (as if pledge(2) calls occured in some serial order). Currently it isn't exactly the case: you could enter in pledge(2), sleep (with current code, the probability to sleep is low: it would be due to malloc(M_WAITOK)), and return 0 (promises applied), and finally have others promises applied (from other thread). On the other side, the problem concerns only consistent behaviour for concurrent pledge(2) calls: if sleep occurs on middle of this syscall, shared structures (ps_pledge or ps_pledgepaths) are still consistents, and restrictions should be still correctly applied. It has to be noted also that some programming languages use threading by default, letting pledge(2) possible use to occurs necessary in one thread (haskell with ghci for example). So just deny pledge(2) usage in threaded context isn't really a solution. About the use of tsleep(9), it was a way proposed to me for resolving the "atomic" view of pledge(2) from userland, with getblk() as example. But as I am far to understand all internals in kernel, I have no problem to look at another way to resolv this. Thanks. -- Sebastien Marie
