[PATCH] smb: client: fix rlist race and missing initialization

[PATCH] smb: client: fix rlist race and missing initialization

[Paulo Alcantara]

TCP_Server_Info.rlist is allocated via kzalloc which zeros both ->next
and ->prev to NULL instead of pointing to itself, making list_empty()
always return false and list_add() dereference a NULL ->prev pointer.

Also, cifs_signal_cifsd_for_reconnect() can be called concurrently
from multiple cifsd threads, allowing the same server's rlist node to
be added twice into the local list, corrupting it.

Fixes: df0e03a4fb94 ("smb: client: fix potential deadlock when reconnecting channels")
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
---
 fs/smb/client/connect.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index b6e98eb31673..bb256b60aa5d 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -174,6 +174,8 @@ cifs_signal_cifsd_for_reconnect(struct TCP_Server_Info *server,
 				nserver = ses->chans[i].server;
 				if (!nserver)
 					continue;
+				if (!list_empty(&nserver->rlist))
+					continue;
 				nserver->srv_count++;
 				list_add(&nserver->rlist, &reco);
 			}
@@ -1823,6 +1825,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
 	spin_lock_init(&tcp_ses->mid_counter_lock);
 	INIT_LIST_HEAD(&tcp_ses->tcp_ses_list);
 	INIT_LIST_HEAD(&tcp_ses->smb_ses_list);
+	INIT_LIST_HEAD(&tcp_ses->rlist);
 	INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request);
 	INIT_DELAYED_WORK(&tcp_ses->reconnect, smb2_reconnect_server);
 	mutex_init(&tcp_ses->reconnect_mutex);
-- 
2.55.0

Re: [PATCH] smb: client: fix rlist race and missing initialization

[Paulo Alcantara]

Paulo Alcantara <pc@manguebit.org> writes:

> TCP_Server_Info.rlist is allocated via kzalloc which zeros both ->next
> and ->prev to NULL instead of pointing to itself, making list_empty()
> always return false and list_add() dereference a NULL ->prev pointer.
>
> Also, cifs_signal_cifsd_for_reconnect() can be called concurrently
> from multiple cifsd threads, allowing the same server's rlist node to
> be added twice into the local list, corrupting it.
>
> Fixes: df0e03a4fb94 ("smb: client: fix potential deadlock when reconnecting channels")
> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
> Cc: Namjae Jeon <linkinjeon@kernel.org>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Bharath SM <bharathsm@microsoft.com>
> Cc: stable@vger.kernel.org

Please ignore it.  Sashiko found out a potential issue with this patch.

I'll rework it and then post v2 soon.