Re: possible deadlock in aio_poll

[Christoph Hellwig]

On Mon, Sep 10, 2018 at 12:41:05AM -0700, syzbot wrote:
> =====================================================
> WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
> 4.19.0-rc2+ #229 Not tainted
> -----------------------------------------------------
> syz-executor2/9399 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> 00000000126506e0 (&ctx->fd_wqh){+.+.}, at: spin_lock
> include/linux/spinlock.h:329 [inline]
> 00000000126506e0 (&ctx->fd_wqh){+.+.}, at: aio_poll+0x760/0x1420
> fs/aio.c:1747
> 
> and this task is already holding:
> 000000002bed6bf6 (&(&ctx->ctx_lock)->rlock){..-.}, at: spin_lock_irq
> include/linux/spinlock.h:354 [inline]
> 000000002bed6bf6 (&(&ctx->ctx_lock)->rlock){..-.}, at: aio_poll+0x738/0x1420
> fs/aio.c:1746
> which would create a new lock dependency:
>  (&(&ctx->ctx_lock)->rlock){..-.} -> (&ctx->fd_wqh){+.+.}

ctx->fd_wqh seems to only exist in userfaultfd, which indeed seems
to do strange open coded waitqueue locking, and seems to fail to disable
irqs.  Something like this should fix it:

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index bfa0ec69f924..356d2b8568c1 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1026,7 +1026,7 @@ static ssize_t userfaultfd_ctx_read(struct 
userfaultfd_ctx *ctx, int no_wait,
        struct userfaultfd_ctx *fork_nctx = NULL;
 
        /* always take the fd_wqh lock before the fault_pending_wqh lock */
-       spin_lock(&ctx->fd_wqh.lock);
+       spin_lock_irq(&ctx->fd_wqh.lock);
        __add_wait_queue(&ctx->fd_wqh, &wait);
        for (;;) {
                set_current_state(TASK_INTERRUPTIBLE);
@@ -1112,13 +1112,13 @@ static ssize_t userfaultfd_ctx_read(struct 
userfaultfd_ctx *ctx, int no_wait,
                        ret = -EAGAIN;
                        break;
                }
-               spin_unlock(&ctx->fd_wqh.lock);
+               spin_unlock_irq(&ctx->fd_wqh.lock);
                schedule();
-               spin_lock(&ctx->fd_wqh.lock);
+               spin_lock_irq(&ctx->fd_wqh.lock);
        }
        __remove_wait_queue(&ctx->fd_wqh, &wait);
        __set_current_state(TASK_RUNNING);
-       spin_unlock(&ctx->fd_wqh.lock);
+       spin_unlock_irq(&ctx->fd_wqh.lock);
 
        if (!ret && msg->event == UFFD_EVENT_FORK) {
                ret = resolve_userfault_fork(ctx, fork_nctx, msg);