#24946 closed defect (fixed)

connection_ap_expire_beginning(): Bug: circuit->purpose == CIRCUIT_PURPOSE_C_GENERAL failed

Reported by: arma Owned by: mikeperry
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

On my onion service, I now get:

Jan 19 18:24:30.165 [warn] connection_ap_expire_beginning(): Bug: circuit->purpose == CIRCUIT_PURPOSE_C_GENERAL failed. The purpose on the circuit was Hidden service: Uploading HS descriptor; it was in state open, path_state new. (on Tor 0.3.3.0-alpha-dev ef148638a1d3b312)

It looks like the clause in question is

    }
    if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL &&
        circ->purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT &&
        circ->purpose != CIRCUIT_PURPOSE_PATH_BIAS_TESTING) {
      log_warn(LD_BUG, "circuit->purpose == CIRCUIT_PURPOSE_C_GENERAL failed. "
               "The purpose on the circuit was %s; it was in state %s, "
               "path_state %s.",
               circuit_purpose_to_string(circ->purpose),
               circuit_state_to_string(circ->state),
               CIRCUIT_IS_ORIGIN(circ) ?
                pathbias_state_to_string(TO_ORIGIN_CIRCUIT(circ)->path_state) :
                "none");
    }

and I'm suspecting that #23101 wanted to add a line here.

Child Tickets

Change History (8)

comment:1 Changed 18 months ago by arma

Owner: set to mikeperry
Status: newassigned

comment:2 Changed 18 months ago by mikeperry

Status: assignedneeds_review

Ok, I fixed this, along with 3 others that I found by looking at every occurrence of CIRCUIT_PURPOSE_C_GENERAL (which CIRCUIT_PURPOSE_C_HSDIR_POST and CIRCUIT_PURPOSE_C_HSDIR_GET were split off from). https://oniongit.eu/mikeperry/tor/commits/bug24946

There is one other instance in count_pending_general_client_circuits(), which is called by circuit_get_open_circ_or_launch() to determine if we have too many non-hidden service client circuits available. If HSDIR retries are already rate limited (I think they are, but I am not 100% sure), then I don't think we need to patch that one. It would be nice if someone who knows could confirm that hsdir attempts/retries are rate limited for v2 and v3 though.

Last edited 18 months ago by mikeperry (previous) (diff)

comment:3 Changed 18 months ago by ffmancera

The patch seems fine for me and about the instance in count_pending_general_client_circuits() called by circuit_get_open_circ_or_launch(), I am not completly sure but I think we don't need to patch it too.

comment:4 Changed 18 months ago by nickm

lgtm; asn, can you take a look?

comment:5 Changed 18 months ago by asn

Mike's fix LGTM too.

Mike any chance we should also be checking for HS_VANGUARD circ->purpose in connection_ap_expire_beginning() too?

WRT mike's question on comment:2: I don't think we have inherent rate-limiting for hsdir queries on hsv2 or hsv3, but I don't expect Tor clients to make many such circuits. Perhaps we should merge the bug fix patch we already have, and maybe open a separate ticket about investigating the potential issue of count_pending_general_client_circuits()?

comment:6 Changed 18 months ago by nickm

merged, but not closing. Mike, can you answer asn's questions above and could one of you open tickets as appropriate?

comment:7 in reply to:  5 Changed 18 months ago by mikeperry

Replying to asn:

Mike's fix LGTM too.

Mike any chance we should also be checking for HS_VANGUARD circ->purpose in connection_ap_expire_beginning() too?

No, this check is only for circuits that can have attempted streams attached. HS_VANGAURD should never have any streams.

WRT mike's question on comment:2: I don't think we have inherent rate-limiting for hsdir queries on hsv2 or hsv3, but I don't expect Tor clients to make many such circuits. Perhaps we should merge the bug fix patch we already have, and maybe open a separate ticket about investigating the potential issue of count_pending_general_client_circuits()?

Ok. #24989.

comment:8 Changed 18 months ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Closing this one per comments above.

Note: See TracTickets for help on using tickets.