From 0507d3f44b910ebe855b527ab894bdb08e33b48d Mon Sep 17 00:00:00 2001
From: Tomasz Gajewski <tomga@wp.pl>
Date: Tue, 18 May 2021 01:08:28 +0200
Subject: [PATCH] ssh_terminal: fixed managing ssh file descriptors

Managing ssh event file descriptors was performed from two different
threads which could cause reallocation of structure used in other thread
in a call to 'poll' function.

Splitted initialization to parts and moved ssh event part into ssh loop.

Issue #4095
---
 repos/gems/src/server/ssh_terminal/server.cc | 41 ++++++++++++++++----
 repos/gems/src/server/ssh_terminal/server.h  | 20 ++++++++--
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/repos/gems/src/server/ssh_terminal/server.cc b/repos/gems/src/server/ssh_terminal/server.cc
index 311aedce8f..9275f6f136 100644
--- a/repos/gems/src/server/ssh_terminal/server.cc
+++ b/repos/gems/src/server/ssh_terminal/server.cc
@@ -48,16 +48,28 @@ Ssh::Terminal_session::Terminal_session(Genode::Registry<Terminal_session> &reg,
 :
 	Element(reg, *this), conn(conn), _event_loop(event_loop)
 {
-	if (pipe(_fds) ||
-		ssh_event_add_fd(_event_loop,
-		                 _fds[0],
-		                 POLLIN,
-		                 write_avail_cb,
-		                 this) != SSH_OK ) {
+	if (pipe(_fds)) {
 		Genode::error("Failed to create wakeup pipe");
 		throw -1;
 	}
 	conn.write_avail_fd = _fds[1];
+
+	_state = PIPE_INITIALIZED;
+}
+
+void Ssh::Terminal_session::initialize_ssh_event_fds()
+{
+	if (_state != PIPE_INITIALIZED ||
+	    ssh_event_add_fd(_event_loop,
+	                     _fds[0],
+	                     POLLIN,
+	                     write_avail_cb,
+	                     this) != SSH_OK) {
+		Genode::error("Failed to initialize ssh event file descriptors");
+		throw -1;
+	}
+
+	_state = SSH_INITIALIZED;
 }
 
 
@@ -596,7 +608,22 @@ void Ssh::Server::loop()
 		{
 			Util::Pthread_mutex::Guard guard(_terminals.mutex());
 
-			/* first remove all stale sessions */
+			/* finish pending initialization of terminal sessions */
+			auto initialize = [&] (Terminal_session &t) {
+				try {
+					if (t._state == Terminal_session::PIPE_INITIALIZED) {
+						t.initialize_ssh_event_fds();
+					}
+				} catch (...) {
+					/* Not sure what to do here - terminal is "almost" attached.
+					   Previously service was denied in that case but as
+					   descriptor handling must be performed in ssh loop thread
+					   it is too late for that. */
+				}
+			};
+			_terminals.for_each(initialize);
+
+			/* remove all stale sessions */
 			auto cleanup = [&] (Session &s) {
 				if (s.terminal_detached) {
 					Terminal_session *p = nullptr;
diff --git a/repos/gems/src/server/ssh_terminal/server.h b/repos/gems/src/server/ssh_terminal/server.h
index f9630793fc..1e33404cac 100644
--- a/repos/gems/src/server/ssh_terminal/server.h
+++ b/repos/gems/src/server/ssh_terminal/server.h
@@ -88,16 +88,30 @@ struct Ssh::Terminal_session : Genode::Registry<Terminal_session>::Element
 
 	int _fds[2] { -1, -1 };
 
+	enum State { UNINITIALIZED,
+	             PIPE_INITIALIZED,
+	             SSH_INITIALIZED } _state = UNINITIALIZED;
+
 	Terminal_session(Genode::Registry<Terminal_session> &reg,
 	                 Ssh::Terminal &conn,
 	                 ssh_event event_loop);
 
 	~Terminal_session()
 	{
-		ssh_event_remove_fd(_event_loop, _fds[0]);
-		close(_fds[0]);
-		close(_fds[1]);
+		switch (_state) {
+		case SSH_INITIALIZED:
+			ssh_event_remove_fd(_event_loop, _fds[0]);
+			[[fallthrough]];
+		case PIPE_INITIALIZED:
+			close(_fds[0]);
+			close(_fds[1]);
+			[[fallthrough]];
+		case UNINITIALIZED:
+			break;
+		}
 	}
+
+	void initialize_ssh_event_fds();
 };