From 346849ac253a08374b9bfca01e2f46a2f90da42f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net>
Date: Thu, 29 Feb 2024 14:49:52 -0500
Subject: [PATCH] Unlock ssh_mutex before calling sftps_recv()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This should fix the sftp hangs...
While FreeBSD will return EDEADLOCK when a mutex would deadlock,
Linux will do what I ask and deadlock.

Since we're not checking the return value of pthread_mutex_lock(),
this would go completely unnoticed on FreeBSD, resulting in the
mutex being unlocked early and all sorts of unprotected accesses
would happen, potentially doing crazy things.  On Linux it just
deadlocks, and taking a peek at the thread backtraces makes the
problem obvious.

So, point to Linux for making life more correct for people who
do incorrect things (locking without checking the return value).

I'd also like to thank nelgin, for sticking with me on this issue
and being my gdb puppet.

And really, this commit should be shared by all the contributors
who... 𝆺𝅥𝅮𝆺𝅥𝅮𝆹𝅥𝅯𝆹𝅥𝅯
---
 src/sbbs3/main.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/sbbs3/main.cpp b/src/sbbs3/main.cpp
index 5e6cb76286..6e47c12e88 100644
--- a/src/sbbs3/main.cpp
+++ b/src/sbbs3/main.cpp
@@ -1957,8 +1957,10 @@ static int crypt_pop_channel_data(sbbs_t *sbbs, char *inbuf, int want, int *got)
 				if (cid == closing_channel)
 					continue;
 				if (cid == sbbs->sftp_channel) {
+					pthread_mutex_unlock(&sbbs->ssh_mutex);
 					if (!sftps_recv(sbbs->sftp_state, reinterpret_cast<uint8_t *>(inbuf), tgot))
 						sbbs->sftp_end();
+					pthread_mutex_lock(&sbbs->ssh_mutex);
 				}
 				else if (cid == sbbs->session_channel) {
 					*got = tgot;
@@ -1975,8 +1977,10 @@ static int crypt_pop_channel_data(sbbs_t *sbbs, char *inbuf, int want, int *got)
 					if (((startup->options & (BBS_OPT_ALLOW_SFTP | BBS_OPT_SSH_ANYAUTH)) == BBS_OPT_ALLOW_SFTP) && ssname && cname && sbbs->sftp_channel == -1 && strcmp(ssname, "sftp") == 0) {
 						if (sbbs->init_sftp(cid)) {
 							if (tgot > 0) {
+								pthread_mutex_lock(&sbbs->ssh_mutex);
 								if (!sftps_recv(sbbs->sftp_state, reinterpret_cast<uint8_t *>(inbuf), tgot))
 									sbbs->sftp_end();
+								pthread_mutex_lock(&sbbs->ssh_mutex);
 							}
 							sbbs->sftp_channel = cid;
 						}
-- 
GitLab