Opened 3 years ago

Last modified 8 months ago

#25312 new defect

circ: We can pick an active circuit that is marked for close

Reported by: dgoulet Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-cmux, tor-circuit, 034-triage-20180328, 034-removed-20180328
Cc: Actual Points:
Parent ID: #25328 Points:
Reviewer: Sponsor:


The issue lies in when we detach cicuits from the cmux.

The function circuitmux_get_first_active_circuit() returns the first active circuit from the given cmux (attached to the channel).

But, when we mark for close a circuit, we don't detach it from the cmux, it is only done "before free" so the result is that cmux->policy->pick_active_circuit() can return a marked for close circuit then a cell is dequeued from it and sent on the wire.

In my experimentation, I only saw END, DROP and TRUNCATED relay commands being sent on the wire from a marked for close circuit. Thus, I don't think that is currently such a big problem but still, not that good I would say.

We should assume that from the time the circuit is marked for close, nothing should go outbound on it from that point on.

Possible solution would be to detach the circuit from the cmux when marked for close or make the active circuit function ignore closed circuit.

Not sure at this point about backport.

Child Tickets

Change History (7)

comment:1 Changed 3 years ago by dgoulet

Datapoint: after one night of running this, I see more type of cells now, DATA, EXTENDED and BEGIN. Thus, this clearly affects any type of circuits.

comment:2 Changed 3 years ago by dgoulet

Parent ID: #25328

comment:3 Changed 3 years ago by nickm

Keywords: 034-triage-20180328 added

comment:4 Changed 3 years ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:5 Changed 3 years ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.

comment:6 Changed 17 months ago by gaba

Owner: dgoulet deleted

Releasing some old tickets.

comment:7 Changed 8 months ago by teor

Status: assignednew

Change tickets that are assigned to nobody to "new".

Note: See TracTickets for help on using tickets.