Skip to content

Commit

Permalink
[backport] fix(libtor): don't leak a file descriptor (#1072)
Browse files Browse the repository at this point in the history
Discovered while trying to understand the reason why we have an abort caused by Android's fdsan.

This is not the reason why we abort, but it seems correct to fix this issue anyway.

While there, notice that stopping to run libtorlinux for every PR (which we just did in 5ebcca2) was wrong. We need to run this workflow for every PR because it runs tests for the libtor integration.

Part of ooni/probe#2405
  • Loading branch information
bassosimone committed Feb 3, 2023
1 parent 5a0a0d6 commit 8a1a8fd
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
1 change: 1 addition & 0 deletions .github/workflows/libtorlinux.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Runs tests for internal/libtor with -tags=ooni_libtor
name: libtorlinux
on:
pull_request:
push:
branches:
- "master"
Expand Down
15 changes: 14 additions & 1 deletion internal/libtor/enabled.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,28 @@ func (p *torProcess) runtor(ctx context.Context, cc net.Conn, args ...string) {
// filedesc is good, os.NewFile shouldn't fail.
filep := os.NewFile(uintptr(filedesc), "")
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.
conn, err := net.FileConn(filep)
if p.simulateFileConnFailure {
err = ErrCannotCreateControlSocket
}
if err != nil {
filep.Close()
p.startErr <- err // nonblocking channel
return
}

// From the documentation of [net.FileConn]:
//
// It is the caller's responsibility to close f when
// finished. Closing c does not affect f, and closing
// f does not affect c.
//
// So, it's safe to close the filep now.
filep.Close()

// In the following we're going to possibly call Close multiple
// times. Let's be very sure that this close is idempotent.
conn = withIdempotentClose(conn)
Expand All @@ -227,7 +240,7 @@ func (p *torProcess) runtor(ctx context.Context, cc net.Conn, args ...string) {
<-ctx.Done()
}()

// Route messages from and to the control connection.
// Route messages to and from the control connection.
go sendrecvThenClose(cc, conn)
go sendrecvThenClose(conn, cc)

Expand Down

0 comments on commit 8a1a8fd

Please sign in to comment.