re: userfaultfd: add new syscall to provide memory externalization

re: userfaultfd: add new syscall to provide memory externalization

[Dan Carpenter]

Hello Andrea Arcangeli,

The patch 126710df097a: "userfaultfd: add new syscall to provide
memory externalization" from May 23, 2015, leads to the following
static checker warning:

	fs/userfaultfd.c:567 userfaultfd_ctx_read()
	error:  locking inconsistency.  We assume 'irq:' is both locked and unlocked at the start.

fs/userfaultfd.c
   505  static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
   506                                      struct uffd_msg *msg)
   507  {
   508          ssize_t ret;
   509          DECLARE_WAITQUEUE(wait, current);
   510          struct userfaultfd_wait_queue *uwq;
   511  
   512          /* always take the fd_wqh lock before the fault_pending_wqh lock */
   513          spin_lock(&ctx->fd_wqh.lock);
   514          __add_wait_queue(&ctx->fd_wqh, &wait);
   515          for (;;) {
   516                  set_current_state(TASK_INTERRUPTIBLE);
   517                  spin_lock(&ctx->fault_pending_wqh.lock);
   518                  uwq = find_userfault(ctx);
   519                  if (uwq) {
   520                          /*
   521                           * The fault_pending_wqh.lock prevents the uwq
   522                           * to disappear from under us.
   523                           *
   524                           * Refile this userfault from
   525                           * fault_pending_wqh to fault_wqh, it's not
   526                           * pending anymore after we read it.
   527                           *
   528                           * Use list_del() by hand (as
   529                           * userfaultfd_wake_function also uses
   530                           * list_del_init() by hand) to be sure nobody
   531                           * changes __remove_wait_queue() to use
   532                           * list_del_init() in turn breaking the
   533                           * !list_empty_careful() check in
   534                           * handle_userfault(). The uwq->wq.task_list
   535                           * must never be empty at any time during the
   536                           * refile, or the waitqueue could disappear
   537                           * from under us. The "wait_queue_head_t"
   538                           * parameter of __remove_wait_queue() is unused
   539                           * anyway.
   540                           */
   541                          list_del(&uwq->wq.task_list);
   542                          __add_wait_queue(&ctx->fault_wqh, &uwq->wq);
   543  
   544                          /* careful to always initialize msg if ret == 0 */
   545                          *msg = uwq->msg;
   546                          spin_unlock(&ctx->fault_pending_wqh.lock);
   547                          ret = 0;
   548                          break;
   549                  }
   550                  spin_unlock(&ctx->fault_pending_wqh.lock);
   551                  if (signal_pending(current)) {
   552                          ret = -ERESTARTSYS;
   553                          break;
   554                  }
   555                  if (no_wait) {
   556                          ret = -EAGAIN;
   557                          break;
   558                  }
   559                  spin_unlock(&ctx->fd_wqh.lock);
   560                  schedule();
   561                  spin_lock_irq(&ctx->fd_wqh.lock);
                                  ^^^
_irq() here.

   562          }
   563          __remove_wait_queue(&ctx->fd_wqh, &wait);
   564          __set_current_state(TASK_RUNNING);
   565          spin_unlock_irq(&ctx->fd_wqh.lock);
                           ^^^^
and here.

   566  
   567          return ret;
   568  }

regards,
dan carpenter