Opened 5 years ago

Last modified 2 years ago

#14900 reopened defect

To link connections only if they ready

Reported by: cypherpunks Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-client correctness needs-interpretation
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

connection_exit_connect_dir and connection_ap_make_link creates linked pair for connections that can be freed before connection_unlink thus invalidates linked_conn.

  connection_link_connections(partner, base_conn);

  if (connection_add(base_conn) < 0) { /* no space, forget it */
    connection_free(base_conn);
    return NULL;
  }
  connection_link_connections(TO_CONN(dirconn), TO_CONN(exitconn));

  if (connection_add(TO_CONN(exitconn))<0) {
    connection_edge_end(exitconn, END_STREAM_REASON_RESOURCELIMIT);
    connection_free(TO_CONN(exitconn));
    connection_free(TO_CONN(dirconn));
    return 0;
  }

  /* link exitconn to circ, now that we know we can use it. */
  exitconn->next_stream = circ->n_streams;
  circ->n_streams = exitconn;

  if (connection_add(TO_CONN(dirconn))<0) {
    connection_edge_end(exitconn, END_STREAM_REASON_RESOURCELIMIT);
    connection_close_immediate(TO_CONN(exitconn));
    connection_mark_for_close(TO_CONN(exitconn));
    connection_free(TO_CONN(dirconn));
    return 0;
  }

If connection_add fails then linked_conn from another connection is broken. (currently it can to fail only if BUFFEREVENTS used by code)

Child Tickets

Change History (13)

comment:1 Changed 5 years ago by cypherpunks

Yes, code (connection_free) can handle this by checking for linked_conn.

log_err(LD_BUG, "Called with conn->linked_conn still set.");

comment:2 Changed 5 years ago by cypherpunks

Resolution: invalid
Status: newclosed

connection_add asserts if not linked without socket.

  tor_assert(SOCKET_OK(conn->s) ||
             conn->linked ||
             (conn->type == CONN_TYPE_AP &&
              TO_EDGE_CONN(conn)->is_dns_request));

You can't to move linking. Bug is unfixable.

comment:3 Changed 5 years ago by arma

Resolution: invalid
Status: closedreopened

You're doing that thing where you ruin your bug reports if nobody answers in the first three days. :(

comment:4 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-final

comment:5 Changed 5 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:6 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:7 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:8 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:9 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:10 Changed 3 years ago by nickm

Keywords: 027-triaged-in added

comment:11 Changed 3 years ago by nickm

Keywords: 027-triaged-in removed

comment:12 Changed 3 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:13 Changed 2 years ago by nickm

Keywords: tor-client correctness needs-interpretation added
Severity: Normal
Note: See TracTickets for help on using tickets.