[PATCH] cifs: Fix server use-after-free in cifs_chan_skip_or_disable()

[PATCH] cifs: Fix server use-after-free in cifs_chan_skip_or_disable()

[Wentao Liang]

When a secondary channel is no longer supported by the server,
cifs_chan_skip_or_disable() drops the channel reference with
cifs_put_tcp_session() and then continues to use the server pointer by
calling cifs_signal_cifsd_for_reconnect() on it and reading its
primary_server pointer. cifs_put_tcp_session() can drop the last
reference of the channel and tear it down, so both the channel and the
primary server (whose reference is also dropped by
cifs_put_tcp_session()) can be freed before they are signaled for
reconnect.

Signal the channel and the primary server and capture the primary
server pointer before dropping the channel reference with
cifs_put_tcp_session().

Fixes: f591062bdbf4 ("cifs: handle servers that still advertise multichannel after disabling")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 fs/smb/client/smb2pdu.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 3bd300347f16..a33f9b9ed32b 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -189,18 +189,19 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
 		spin_unlock(&ses->chan_lock);
 
 		/*
-		 * the above reference of server by channel
-		 * needs to be dropped without holding chan_lock
-		 * as cifs_put_tcp_session takes a higher lock
-		 * i.e. cifs_tcp_ses_lock
+		 * signal the channel and its primary server to
+		 * reconnect before dropping the above reference of
+		 * server by channel, which is done without holding
+		 * chan_lock as cifs_put_tcp_session takes a higher
+		 * lock i.e. cifs_tcp_ses_lock
 		 */
-		cifs_put_tcp_session(server, from_reconnect);
-
 		cifs_signal_cifsd_for_reconnect(server, false);
 
 		/* mark primary server as needing reconnect */
 		pserver = server->primary_server;
 		cifs_signal_cifsd_for_reconnect(pserver, false);
+
+		cifs_put_tcp_session(server, from_reconnect);
 skip_terminate:
 		return -EHOSTDOWN;
 	}
-- 
2.34.1

Re: [PATCH] cifs: Fix server use-after-free in cifs_chan_skip_or_disable()

[Paulo Alcantara]

Wentao Liang <vulab@iscas.ac.cn> writes:

> When a secondary channel is no longer supported by the server,
> cifs_chan_skip_or_disable() drops the channel reference with
> cifs_put_tcp_session() and then continues to use the server pointer by
> calling cifs_signal_cifsd_for_reconnect() on it and reading its
> primary_server pointer. cifs_put_tcp_session() can drop the last
> reference of the channel and tear it down, so both the channel and the
> primary server (whose reference is also dropped by
> cifs_put_tcp_session()) can be freed before they are signaled for
> reconnect.
>
> Signal the channel and the primary server and capture the primary
> server pointer before dropping the channel reference with
> cifs_put_tcp_session().
> ...

Applied.