On Mon, Sep 09, 2024 at 10:50:04AM +0200, Christian Brauner wrote: > Hey, > > This is another round of Christian's asking sus questions about kernel > apis. I asked them a few people and generally the answers I got was > "Good question, I don't know." or the reasoning varied a lot. So I take > it I'm not the only one with that question. > > I was looking at a potential epoll() bug and it got me thinking about > dos & don'ts for put_user()/copy_from_user() and related helpers as > epoll does acquire the epoll mutex and then goes on to loop over a list > of ready items and calls __put_user() for each item. Granted, it only > puts a __u64 and an integer but still that seems adventurous to me and I > wondered why. > > Generally, new vfs apis always try hard to call helpers that copy to or > from userspace without any locks held as my understanding has been that > this is best practice as to avoid risking taking page faults while > holding a mutex or semaphore even though that's supposedly safe. > > Is this understanding correct? And aside from best practice is it in > principle safe to copy to or from userspace with sleeping locks held? You do realize that e.g. write(2) will copy from userland with a sleeping lock (->i_rwsem) held, right? Inevitably so. It really depends upon the lock in question; sure, milder locking environment is generally less headache, but that's about it. You can't do that under page lock. You can do that under ->i_rwsem. As for the epoll mutex... do we ever have it nested inside anything taken on #PF paths? I don't see anything of that sort, but I might've missed something...
