fix(android): avoid fdsan crash in tor code (#1070)

This diff patches tor to avoid closing the controller socket twice. See ooni/probe#2405.
ooni · Feb 3, 2023 · a904ba5 · a904ba5
1 parent 8e1bed1
commit a904ba5
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 1 deletion.
diff --git a/CDEPS/tor/003.patch 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
@@ -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
@@ -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
@@ -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