Opened 6 years ago

Closed 6 years ago

#9400 closed defect (fixed)

double-close in socketpair replacement function on failure

Reported by: nickm Owned by:
Priority: Low Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay 024-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Our socketpair replacement function (only used on Windows), in its clean-up-on-failure code, potentially does a double-close of the listener socket. Additionally, it doesn't use the right macros for invalid sockets.

Found by coverity.

Child Tickets

Change History (7)

comment:1 Changed 6 years ago by nickm

Status: newneeds_review

Fix in branch "bug9400_024", which merges cleanly into 0.2.4 and master. Please review.

comment:2 Changed 6 years ago by arma

Is this because tor_socket_t is unsigned on Windows? Or some other issue? I don't understand the fix because I don't understand the bug.

comment:3 Changed 6 years ago by nickm

031e695aa50f24000ec112d535636807167be436 is cosmetic; that's how we're supposed to be handling invalid sockets throughout our code (because on windows the correct check is "== -1").

1f4dcc957bc8006addcfae4560ab4f8888781ded is the bugfix.... or at least, it was supposed to be. Sorry there! I've added a new commit to have the actual fix. :/ (That's the double-close.)

comment:4 Changed 6 years ago by arma

Don't we want to close the listener when success?

comment:5 Changed 6 years ago by nickm

I believe we still do?

comment:6 Changed 6 years ago by arma

Oh! The close is still there. I missed it.

Patch looks fine.

comment:7 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Squashed and merged; thanks!

Note: See TracTickets for help on using tickets.