[tor-bugs] #7472 [Tor]: Audit all calls to connection_mark_for_close()
Tor Bug Tracker & Wiki
blackhole at torproject.org
Wed Nov 14 14:46:35 UTC 2012
#7472: Audit all calls to connection_mark_for_close()
--------------------+-------------------------------------------------------
Reporter: andrea | Owner: andrea
Type: task | Status: new
Priority: normal | Milestone: Tor: 0.2.4.x-final
Component: Tor | Version: Tor: 0.2.4.5-alpha
Keywords: | Parent:
Points: | Actualpoints:
--------------------+-------------------------------------------------------
Comment(by nickm):
Replying to [comment:1 andrea]:
> I've attached my analysis of all occurrences of
connection_mark_for_close(); the ones potentially meriting a closer look
are:
>
> connection.c:2099: connection_mark_for_close(conn);
> - This is in connection_mark_all_noncontrol_connections(), could be
> an erroneous close of an orconn. What calls this?
It's used when the DisableNetwork option is turned on while Tor is
running.
> connection.c:2804: connection_mark_for_close(linked);
> - Also in connection_handle_read_impl(); tries to flush from linked
conn
> and then connection_flushed_some() fails. Could possibly be an
orconn
> (when do linked conns get created?); comments suggest this case might
be
> a can't-happen, though.
Linked connections are currently always an edge (ap or exit) connection
linked to a directory connection.
> connection.c:3051: connection_mark_for_close(conn);
> - This is in connection_handle_read_cb(), and gets called if
> connection_process_inbuf() fails; could be an orconn?
> Comment suggests this might be a can't-happen.
oops. That comment is asking, "Should the second argument to
connection_process_inbuf really always be 1 here"? It's entirely possible
for connection_process_inbuf() to return -1 on an orconn; check out
connection_or_process_inbuf() for examples.
> connection.c:3063: connection_mark_for_close(conn);
> - This is closely parallel to the above case, but in the
> connection_handle_write_cb() function.
>
> connection.c:3120: connection_mark_for_close(conn);
> - This is the error-handling case in connection_handle_event_cb(); if
the
> error is during connect and conn is an orconn it will call
> connection_or_connect_failed(), but it looks like it probably should
handle
> other orconn cases.
Yeah; note that the last 3 are bufferevent-specific.
> connection.c:3270: connection_mark_for_close(conn);
> - Error with getsockopt() in connection_handle_write_impl(); this
> should probably call connection_or_notify_error() too if it's an
orconn.
agreed
> connection.c:3391: connection_mark_for_close(conn);
> - I *think* this case can't be an *active* orconn, but it might be
worth a
> test here anyway. This one can only happen if
> !(connection_speaks_cells(conn) && conn->state >
> OR_CONN_STATE_PROXY_HANDSHAKING)
Right. connection_speaks_cells() is an alias for "is an orconn" right
now. So any orconn, other than one doing a HTTP or SOCKS handshake with
proxy, will hit the first clause of the conditional statement.
> connection.c:3538: connection_mark_for_close(conn);
> - Error in write_to_buf() or write_to_buf_zlib() from
> connection_write_to_buf_impl_(); this should check for orconn, I
think.
Yes. In practice, I don't think these functions can fail, but f they're
documented to have an error return, we should check for it.
> main.c:731: connection_mark_for_close(conn);
> - This is conn_read_callback() when connection_handle_read() returns
> an error; can this ever happen for an orconn?
> main.c:769: connection_mark_for_close(conn);
> - This is conn_write_callback() when connection_handle_write() returns
> an error; can this ever happen for an orconn?
These two should never happen for any kind of connection, since they only
happen if the connection is not already marked, and
connection_handle_{read,write} are supposed to mark the connection every
time they return -1. But that said, there's no reason to think that a bug
where we forget to mark an orconn is less likely than a bug where we
forget to mark some other kind of conn.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7472#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list