Opened 4 years ago

Closed 2 years ago

#9122 closed defect (fixed)

Recent regression causing confparse error

Reported by: atagar Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client 023-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Hi Nick. For a few weeks now our jenkins tests for stem/tor integration have been busted. Turns out that they caught our first tor bug, yay!

I did a bisect and narrowed the regression to the merge on 5/28 of commit d3125a3...

2613c98 bad (6/16/13)
7159e19 bad (6/5/13)
4d7ac69 bad (5/29/13)
97d1caa bad (5/28/13)
d3125a3 just plain busted (fixed in the next commit)
e7134c2 good (5/24/13)
cb488f9 good (5/20/13)

To repro the issue kick off a tor instance then issue an invalid LOADCONF. Here's an example...

% telnet localhost 9051
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
AUTHENTICATE
250 OK
+LOADCONF
ContactInfo confloaded
.
Connection closed by foreign host.

Tor will fail with...

Jun 23 13:03:25.000 [err] config_is_same(): Bug: src/or/confparse.c:983: config_is_same: Assertion fmt && o2 failed; aborting.
src/or/confparse.c:983 config_is_same: Assertion fmt && o2 failed; aborting.

Cheers! -Damian

Child Tickets

Change History (8)

comment:1 Changed 4 years ago by rransom

This looks related to #9124 (which is also about an attempt to set the ContactInfo option crashing Tor).

comment:2 Changed 4 years ago by nickm

  • Keywords tor-client 023-backport added
  • Milestone set to Tor: 0.2.4.x-final
  • Status changed from new to needs_review

Looks like you don't actually need to set ContactInfo. Passing an empty configuration to LOADCONF will do fine too.

It comes down to the the call in hanle_control_loadconf:

  retval = options_init_from_string(NULL, body, CMD_RUN_TOR, NULL, &errstring);

That initial NULL means "don't use any defaults", which is incorrect on so many levels. The underlying bug exists back to 0.2.3, and is probably causing some weird behavior for somebody somewhere.

"bug9122" has an apparent fix against 0.2.4. "bug9122_023" is a backport of that commit to 0.2.3. Please review?

comment:3 Changed 4 years ago by atagar

Hi Nick. I'm not familiar enough with the codebase to review your patches, but for what it's worth looks good to me. Stem's integ tests pass against your bug9122 branch and I did some manual testing of LOADCONF's behaviour. Thanks for the fix!

comment:4 Changed 4 years ago by nickm

  • Milestone changed from Tor: 0.2.4.x-final to Tor: 0.2.3.x-final

Okay. It seems straightforward to me, so I'm rebasing it onto the right branch, and merging it into 0.2.4 and later.

Bumping down to 0.2.3 for possible backport.

comment:5 Changed 4 years ago by nickm

The fix here caused #9295.

comment:6 Changed 4 years ago by nickm

If we merge this to 0.2.3, merge my "bug9295_023" as well.

comment:7 Changed 2 years ago by arma

I recommend abandoning the 0.2.3 backport notion -- 0.2.3.x is about dead now.

comment:8 Changed 2 years ago by nickm

  • Milestone changed from Tor: 0.2.3.x-final to Tor: 0.2.4.x-final
  • Resolution set to fixed
  • Status changed from needs_review to closed

Agreed.

Note: See TracTickets for help on using tickets.