Re: [PATCH] CIFS: Fix retry mid list corruption on reconnects

From: Greg KH <greg@kroah.com>
To: Pavel Shilovsky <piastryyy@gmail.com>
Cc: stable@vger.kernel.org, Ronnie Sahlberg <lsahlber@redhat.com>,
	David Wysochanski <dwysocha@redhat.com>,
	Steve French <smfrench@gmail.com>
Subject: Re: [PATCH] CIFS: Fix retry mid list corruption on reconnects
Date: Fri, 8 Nov 2019 17:15:25 +0100	[thread overview]
Message-ID: <20191108161525.GE761587@kroah.com> (raw)
In-Reply-To: <20191106181635.25327-1-pshilov@microsoft.com>

On Wed, Nov 06, 2019 at 10:16:35AM -0800, Pavel Shilovsky wrote:
> Commit abe57073d08c1 ("CIFS: Fix retry mid list corruption on reconnects") upstream.
> 
> 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.
> 
> Cc: Stable <stable@vger.kernel.org> # 5.3.x
> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Reviewed-and-tested-by: David Wysochanski <dwysocha@redhat.com>
> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
>  fs/cifs/connect.c   | 10 +++++++++-
>  fs/cifs/transport.c | 42 +++++++++++++++++++++++-------------------
>  2 files changed, 32 insertions(+), 20 deletions(-)

Now queued up, thanks.

greg k-h