On Mon, Sep 9, 2024, at 09:18, Christian Brauner wrote:
> On Mon, Sep 09, 2024 at 10:50:10AM GMT, Christian Brauner wrote:
>>
>> 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?
I would be very suspicious if it's an actual __put_user() rather
than the normal put_user() since at least on x86 that skips the
__might_fault() instrumentation.
With the normal put_user() at least I would expect the
might_lock_read(¤t->mm->mmap_lock) instrumentation
in __might_fault() to cause a lockdep splat if you are holding
a mutex that is also required during a page fault, which
in turn would deadlock if your __user pointer is paged out.
I have not seen that particular lockdep output, but I imagine
that is why VFS code tends to avoids the put_user() inside
of a mutex, while code that is nowhere near paging gets away
with it.
Arnd
