linux-kernel - Re: [syzbot] BUG: sleeping function called from invalid context in _copy_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Aug 2021 13:37:08 -0700
From:   Shoaib Rao <rao.shoaib@...cle.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Dmitry Vyukov <dvyukov@...gle.com>,
        syzbot <syzbot+8760ca6c1ee783ac4abd@...kaller.appspotmail.com>,
        andrii@...nel.org, ast@...nel.org, bpf@...r.kernel.org,
        christian.brauner@...ntu.com, cong.wang@...edance.com,
        daniel@...earbox.net, davem@...emloft.net, edumazet@...gle.com,
        jamorris@...ux.microsoft.com, john.fastabend@...il.com,
        kafai@...com, kpsingh@...nel.org, kuba@...nel.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        netdev@...r.kernel.org, shuah@...nel.org, songliubraving@...com,
        syzkaller-bugs@...glegroups.com, yhs@...com
Subject: Re: [syzbot] BUG: sleeping function called from invalid context in
 _copy_to_iter


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