From a904ba572fece7785db101f8c4af3e7b7794914b Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Fri, 3 Feb 2023 12:33:46 +0100
Subject: [PATCH] fix(android): avoid fdsan crash in tor code (#1070)

This diff patches tor to avoid closing the controller socket twice.

See https://github.com/ooni/probe/issues/2405.
---
 CDEPS/tor/003.patch                       | 18 ++++++++++++++++++
 internal/cmd/buildtool/android_test.go    | 20 ++++++++++++++++++++
 internal/cmd/buildtool/linuxcdeps_test.go |  5 +++++
 internal/libtor/enabled.go                |  2 +-
 4 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 CDEPS/tor/003.patch

diff --git a/CDEPS/tor/003.patch b/CDEPS/tor/003.patch
new file mode 100644
index 0000000000..8b3fdee4a1
--- /dev/null
+++ b/CDEPS/tor/003.patch
@@ -0,0 +1,18 @@
+diff --git a/src/feature/api/tor_api.c b/src/feature/api/tor_api.c
+index 88e91ebfd5..2773949264 100644
+--- a/src/feature/api/tor_api.c
++++ b/src/feature/api/tor_api.c
+@@ -131,9 +131,13 @@ tor_main_configuration_free(tor_main_configuration_t *cfg)
+     }
+     raw_free(cfg->argv_owned);
+   }
++  /* See https://gitlab.torproject.org/tpo/core/tor/-/issues/40747 to
++     understand why we're not closing the socket here. */
++  /*
+   if (SOCKET_OK(cfg->owning_controller_socket)) {
+     raw_closesocket(cfg->owning_controller_socket);
+   }
++  */
+   raw_free(cfg);
+ }
+
diff --git a/internal/cmd/buildtool/android_test.go b/internal/cmd/buildtool/android_test.go
index c5c27db3c9..0f04b13115 100644
--- a/internal/cmd/buildtool/android_test.go
+++ b/internal/cmd/buildtool/android_test.go
@@ -1666,6 +1666,11 @@ func TestAndroidBuildCdepsTor(t *testing.T) {
 			Argv: []string{
 				"git", "apply", faketopdir + "/CDEPS/tor/002.patch",
 			},
+		}, {
+			Env: []string{},
+			Argv: []string{
+				"git", "apply", faketopdir + "/CDEPS/tor/003.patch",
+			},
 		}, {
 			Env: []string{
 				"AS=" + fakeBinPath + "/armv7a-linux-androideabi21-clang",
@@ -1737,6 +1742,11 @@ func TestAndroidBuildCdepsTor(t *testing.T) {
 			Argv: []string{
 				"git", "apply", faketopdir + "/CDEPS/tor/002.patch",
 			},
+		}, {
+			Env: []string{},
+			Argv: []string{
+				"git", "apply", faketopdir + "/CDEPS/tor/003.patch",
+			},
 		}, {
 			Env: []string{
 				"AS=" + fakeBinPath + "/aarch64-linux-android21-clang",
@@ -1808,6 +1818,11 @@ func TestAndroidBuildCdepsTor(t *testing.T) {
 			Argv: []string{
 				"git", "apply", faketopdir + "/CDEPS/tor/002.patch",
 			},
+		}, {
+			Env: []string{},
+			Argv: []string{
+				"git", "apply", faketopdir + "/CDEPS/tor/003.patch",
+			},
 		}, {
 			Env: []string{
 				"AS=" + fakeBinPath + "/i686-linux-android21-clang",
@@ -1879,6 +1894,11 @@ func TestAndroidBuildCdepsTor(t *testing.T) {
 			Argv: []string{
 				"git", "apply", faketopdir + "/CDEPS/tor/002.patch",
 			},
+		}, {
+			Env: []string{},
+			Argv: []string{
+				"git", "apply", faketopdir + "/CDEPS/tor/003.patch",
+			},
 		}, {
 			Env: []string{
 				"AS=" + fakeBinPath + "/x86_64-linux-android21-clang",
diff --git a/internal/cmd/buildtool/linuxcdeps_test.go b/internal/cmd/buildtool/linuxcdeps_test.go
index 2d0e46d42f..999a5c57a8 100644
--- a/internal/cmd/buildtool/linuxcdeps_test.go
+++ b/internal/cmd/buildtool/linuxcdeps_test.go
@@ -321,6 +321,11 @@ func TestLinuxCdepsBuildMain(t *testing.T) {
 			Argv: []string{
 				"git", "apply", faketopdir + "/CDEPS/tor/002.patch",
 			},
+		}, {
+			Env: []string{},
+			Argv: []string{
+				"git", "apply", faketopdir + "/CDEPS/tor/003.patch",
+			},
 		}, {
 			Env: []string{
 				"CFLAGS=-D_FORTIFY_SOURCE=2 -fstack-protector-strong -fstack-clash-protection -fPIC -fsanitize=bounds -fsanitize-undefined-trap-on-error -O2",
diff --git a/internal/libtor/enabled.go b/internal/libtor/enabled.go
index ec1ba579c1..cd20044d0f 100644
--- a/internal/libtor/enabled.go
+++ b/internal/libtor/enabled.go
@@ -207,7 +207,7 @@ func (p *torProcess) runtor(ctx context.Context, cc net.Conn, args ...string) {
 	runtimex.Assert(filep != nil, "os.NewFile should not fail")
 
 	// Create a new net.Conn using a copy of the underlying
-	// file descriptor as documented above.
+	// file descriptor using net.FileConn (see below).
 	conn, err := net.FileConn(filep)
 	if p.simulateFileConnFailure {
 		err = ErrCannotCreateControlSocket