# Re: \[syzbot\] BUG: sleeping function called from invalid context in \_copy\_to\_iter

- From: Shoaib Rao \<hidden\>
- Date: 2021-08-09 20:37:41 
- List: netdev
- Page: https://ratatoskr.run/netdev/2021/08/1472374
- Thread: https://ratatoskr.run/bpf/2021/08/1471358/t.md

```text

On 8/9/21 1:16 PM, Al Viro wrote:
> On Mon, Aug 09, 2021 at 08:04:40PM +0000, Al Viro wrote:
>> On Mon, Aug 09, 2021 at 12:40:03PM -0700, Shoaib Rao wrote:
>>
>>> Page faults occur all the time, the page may not even be in the cache or the
>>> mapping is not there (mmap), so I would not consider this a bug. The code
>>> should complain about all other calls as they are also copying  to user
>>> pages. I must not be following some semantics for the code to be triggered
>>> but I can not figure that out. What is the recommended interface to do user
>>> copy from kernel?
>> 	What are you talking about?  Yes, page faults happen.  No, they
>> must not be triggered in contexts when you cannot afford going to sleep.
>> In particular, you can't do that while holding a spinlock.
>>
>> 	There are things that can't be done under a spinlock.  If your
>> commit is attempting that, it's simply broken.
> ... in particular, this
>
> +#if IS_ENABLED(CONFIG_AF_UNIX_OOB)
> +               mutex_lock(&u->iolock);
> +               unix_state_lock(sk);
> +
> +               err = unix_stream_recv_urg(state);
> +
> +               unix_state_unlock(sk);
> +               mutex_unlock(&u->iolock);
> +#endif
>
> is 100% broken, since you *are* attempting to copy data to userland between
> spin_lock(&unix_sk(s)->lock) and spin_unlock(&unix_sk(s)->lock).

Yes, but why are we calling it unix_state_lock() why not unix_state_spinlock() ?

I have tons of experience doing kernel coding and you can never ever cover everything, that is why I wanted to root cause the issue instead of just turning off the check.

Imagine you or Eric make a mistake and break the kernel, how would you guys feel if I were to write a similar email?

Shoaib

>
> You can't do blocking operations under a spinlock.  And copyout is inherently
> a blocking operation - it can require any kind of IO to complete.  If you
> have the destination (very much valid - no bad addresses there) in the middle
> of a page mmapped from a file and currently not paged in, you *must* read
> the current contents of the page, at least into the parts of page that
> are not going to be overwritten by your copyout.  No way around that.  And
> that can involve any kind of delays and any amount of disk/network/whatnot
> traffic.
>
> You fundamentally can not do that kind of thing without giving the CPU up.
> And under a spinlock you are not allowed to do that.
>
> In the current form that commit is obviously broken.
I am
```