From f236e99b5c97ec3fc8d7053dcf9c4abf7fabdb01 Mon Sep 17 00:00:00 2001
From: Alexander Boettcher <alexander.boettcher@genode-labs.com>
Date: Wed, 28 Apr 2021 08:49:23 +0200
Subject: [PATCH] ssh_terminal: avoid deadlock of EP and pthread.0

pthread.0 acquires a write buffer mutex and calls potentially
blocking fs operations. The EP thread handles session requests and tries to
acquire the same write buffer lock. IO progress events for pthread.0 are
handled by the EP thread, which however is blocking on the write buffer mutex.

The commit uses two write buffers, one which is filled by the EP and a second
which is used by pthread.0. The two buffers are swapped protected by a mutex
without invoking blocking fs operations.

Issue #4095
---
 .../server/ssh_terminal/session_component.h   | 18 +++---
 repos/gems/src/server/ssh_terminal/terminal.h | 61 +++++++++++++------
 2 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/repos/gems/src/server/ssh_terminal/session_component.h b/repos/gems/src/server/ssh_terminal/session_component.h
index 7b0bb589a0..c3c91ad86c 100644
--- a/repos/gems/src/server/ssh_terminal/session_component.h
+++ b/repos/gems/src/server/ssh_terminal/session_component.h
@@ -86,16 +86,16 @@ class Terminal::Session_component : public Genode::Rpc_object<Session, Session_c
 		Genode::size_t _write(Genode::size_t num)
 		{
 			ssize_t written = 0;
-			Libc::with_libc([&] () {
-				char *buf = _io_buffer.local_addr<char>();
-				num = Genode::min(num, _io_buffer.size());
-				written = Ssh::Terminal::write(buf, num);
 
-				if (written < 0) {
-					Genode::error("write error, dropping data");
-					written = 0;
-				}
-			});
+			char *buf = _io_buffer.local_addr<char>();
+			num = Genode::min(num, _io_buffer.size());
+			written = Ssh::Terminal::write(buf, num);
+
+			if (written < 0) {
+				Genode::error("write error, dropping data");
+				written = 0;
+			}
+
 			return written;
 		}
 };
diff --git a/repos/gems/src/server/ssh_terminal/terminal.h b/repos/gems/src/server/ssh_terminal/terminal.h
index 8d6ff22f90..ff7f2d5f94 100644
--- a/repos/gems/src/server/ssh_terminal/terminal.h
+++ b/repos/gems/src/server/ssh_terminal/terminal.h
@@ -37,15 +37,16 @@ namespace Ssh
 
 class Ssh::Terminal
 {
-	public:
-
-		Util::Buffer<4096u> read_buf { };
-
-		int write_avail_fd { -1 };
-
 	private:
 
-		Util::Buffer<4096u> _write_buf { };
+		typedef Util::Buffer<4096u> Buffer;
+
+		Mutex   _write_buf_swap    { };
+
+		Buffer  _write_buf_a       { };
+		Buffer  _write_buf_b       { };
+		Buffer *_write_buf_ep      { &_write_buf_a };
+		Buffer *_write_buf_pthread { &_write_buf_b };
 
 		::Terminal::Session::Size _size { 0, 0 };
 
@@ -60,6 +61,10 @@ class Ssh::Terminal
 
 	public:
 
+		Buffer read_buf { };
+
+		int write_avail_fd { -1 };
+
 		/**
 		 * Constructor
 		 */
@@ -159,15 +164,27 @@ class Ssh::Terminal
 		 */
 		void send(ssh_channel channel)
 		{
-			Mutex::Guard guard(_write_buf.mutex());
+			{
+				/* swap write buffers if current is empty */
+				Mutex::Guard guard(_write_buf_swap);
 
-			if (!_write_buf.read_avail()) { return; }
+				if (!_write_buf_pthread->read_avail()) {
+					auto buffer_tmp    = _write_buf_ep;
+					_write_buf_ep      = _write_buf_pthread;
+					_write_buf_pthread = buffer_tmp;
+				}
+			}
+
+			/* write buffer for pthread is used w/o mutex, it's not used by EP */
+			Buffer &write_buf = *_write_buf_pthread;
+
+			if (!write_buf.read_avail()) { return; }
 
 			/* ignore send request */
 			if (!channel || !ssh_channel_is_open(channel)) { return; }
 
-			char const *src     = _write_buf.content();
-			size_t const len    = _write_buf.read_avail();
+			char const *src     = write_buf.content();
+			size_t const len    = write_buf.read_avail();
 			/* XXX we do not handle partial writes */
 			int const num_bytes = ssh_channel_write(channel, src, len);
 
@@ -176,7 +193,7 @@ class Ssh::Terminal
 			}
 
 			if (++_pending_channels >= _attached_channels) {
-				_write_buf.reset();
+				write_buf.reset();
 			}
 
 			/* at this point the client might have disconnected */
@@ -214,24 +231,28 @@ class Ssh::Terminal
 		 */
 		size_t write(char const *src, Genode::size_t src_len)
 		{
-			Mutex::Guard g(_write_buf.mutex());
-
 			size_t num_bytes = 0;
-			size_t i = 0;
-			Libc::with_libc([&] {
-				while (_write_buf.write_avail() > 0 && i < src_len) {
+
+			{
+				Mutex::Guard guard(_write_buf_swap);
+
+				Buffer &write_buf = *_write_buf_ep;
+
+				size_t i = 0;
+
+				while (write_buf.write_avail() > 0 && i < src_len) {
 
 					char c = src[i];
 					if (c == '\n') {
-						_write_buf.append('\r');
+						write_buf.append('\r');
 					}
 
-					_write_buf.append(c);
+					write_buf.append(c);
 					num_bytes++;
 
 					i++;
 				}
-			});
+			}
 
 			/* wake the event loop up */
 			Libc::with_libc([&] {