Opened 5 years ago

Closed 5 years ago

#11302 closed defect (fixed)

connection_handle_write_impl() should handle orconns properly if getsockopt() fails

Reported by: andrea Owned by: andrea
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version: Tor: 0.2.5.3-alpha
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The function connection_handle_write_impl() in connection.c contains this code:

/* Sometimes, "writable" means "connected". */
if (connection_state_is_connecting(conn)) {
  if (getsockopt(conn->s, SOL_SOCKET, SO_ERROR, (void*)&e, &len) < 0) {
    log_warn(LD_BUG, "getsockopt() syscall failed");
    if (CONN_IS_EDGE(conn))
      connection_edge_end_errno(TO_EDGE_CONN(conn));
    connection_mark_for_close(conn);
    return -1;
  }

This might close an orconn out from under the channel layer improperly. It should test for orconns and call connection_or_connect_failed() in that case rather than connection_mark_for_close() directly. Created pursuant to connection_mark_for_close() audit task #7472.

Child Tickets

Change History (4)

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

comment:2 Changed 5 years ago by andrea

Status: newneeds_review

Proposed fix in my bug11302 branch; it doesn't appear to break anything, but since this is in an apparent can't-happen case I can't confirm that it fixes anything in testing.

comment:3 Changed 5 years ago by nickm

Sure, this seems correct. Merging to 0.2.6

comment:4 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.