1. 25 Oct, 2019 3 commits
    • Dave Wysochanski's avatar
      cifs: Fix cifsInodeInfo lock_sem deadlock when reconnect occurs · d46b0da7
      Dave Wysochanski authored
      There's a deadlock that is possible and can easily be seen with
      a test where multiple readers open/read/close of the same file
      and a disruption occurs causing reconnect.  The deadlock is due
      a reader thread inside cifs_strict_readv calling down_read and
      obtaining lock_sem, and then after reconnect inside
      cifs_reopen_file calling down_read a second time.  If in
      between the two down_read calls, a down_write comes from
      another process, deadlock occurs.
      
              CPU0                    CPU1
              ----                    ----
      cifs_strict_readv()
       down_read(&cifsi->lock_sem);
                                     _cifsFileInfo_put
                                        OR
                                     cifs_new_fileinfo
                                      down_write(&cifsi->lock_sem);
      cifs_reopen_file()
       down_read(&cifsi->lock_sem);
      
      Fix the above by changing all down_write(lock_sem) calls to
      down_write_trylock(lock_sem)/msleep() loop, which in turn
      makes the second down_read call benign since it will never
      block behind the writer while holding lock_sem.
      Signed-off-by: default avatarDave Wysochanski <dwysocha@redhat.com>
      Suggested-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      Reviewed--by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      Reviewed-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
      d46b0da7
    • Pavel Shilovsky's avatar
      CIFS: Fix use after free of file info structures · 1a67c415
      Pavel Shilovsky authored
      Currently the code assumes that if a file info entry belongs
      to lists of open file handles of an inode and a tcon then
      it has non-zero reference. The recent changes broke that
      assumption when putting the last reference of the file info.
      There may be a situation when a file is being deleted but
      nothing prevents another thread to reference it again
      and start using it. This happens because we do not hold
      the inode list lock while checking the number of references
      of the file info structure. Fix this by doing the proper
      locking when doing the check.
      
      Fixes: 487317c9 ("cifs: add spinlock for the openFileList to cifsInodeInfo")
      Fixes: cb248819 ("cifs: use cifsInodeInfo->open_file_lock while iterating to avoid a panic")
      Cc: Stable <stable@vger.kernel.org>
      Reviewed-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      Signed-off-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      1a67c415
    • Pavel Shilovsky's avatar
      CIFS: Fix retry mid list corruption on reconnects · abe57073
      Pavel Shilovsky authored
      When the client hits reconnect it iterates over the mid
      pending queue marking entries for retry and moving them
      to a temporary list to issue callbacks later without holding
      GlobalMid_Lock. In the same time there is no guarantee that
      mids can't be removed from the temporary list or even
      freed completely by another thread. It may cause a temporary
      list corruption:
      
      [  430.454897] list_del corruption. prev->next should be ffff98d3a8f316c0, but was 2e885cb266355469
      [  430.464668] ------------[ cut here ]------------
      [  430.466569] kernel BUG at lib/list_debug.c:51!
      [  430.468476] invalid opcode: 0000 [#1] SMP PTI
      [  430.470286] CPU: 0 PID: 13267 Comm: cifsd Kdump: loaded Not tainted 5.4.0-rc3+ #19
      [  430.473472] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
      [  430.475872] RIP: 0010:__list_del_entry_valid.cold+0x31/0x55
      ...
      [  430.510426] Call Trace:
      [  430.511500]  cifs_reconnect+0x25e/0x610 [cifs]
      [  430.513350]  cifs_readv_from_socket+0x220/0x250 [cifs]
      [  430.515464]  cifs_read_from_socket+0x4a/0x70 [cifs]
      [  430.517452]  ? try_to_wake_up+0x212/0x650
      [  430.519122]  ? cifs_small_buf_get+0x16/0x30 [cifs]
      [  430.521086]  ? allocate_buffers+0x66/0x120 [cifs]
      [  430.523019]  cifs_demultiplex_thread+0xdc/0xc30 [cifs]
      [  430.525116]  kthread+0xfb/0x130
      [  430.526421]  ? cifs_handle_standard+0x190/0x190 [cifs]
      [  430.528514]  ? kthread_park+0x90/0x90
      [  430.530019]  ret_from_fork+0x35/0x40
      
      Fix this by obtaining extra references for mids being retried
      and marking them as MID_DELETED which indicates that such a mid
      has been dequeued from the pending list.
      
      Also move mid cleanup logic from DeleteMidQEntry to
      _cifs_mid_q_entry_release which is called when the last reference
      to a particular mid is put. This allows to avoid any use-after-free
      of response buffers.
      
      The patch needs to be backported to stable kernels. A stable tag
      is not mentioned below because the patch doesn't apply cleanly
      to any actively maintained stable kernel.
      Reviewed-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
      Reviewed-and-tested-by: default avatarDavid Wysochanski <dwysocha@redhat.com>
      Signed-off-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
      Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
      abe57073
  2. 21 Oct, 2019 4 commits
  3. 20 Oct, 2019 6 commits
  4. 19 Oct, 2019 27 commits