Opened 5 years ago

Closed 5 years ago

#11048 closed defect (fixed)

Thread-safety in log_backtrace()

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: #11046 Points:
Reviewer: Sponsor:

Description

On #9299, cypherpunks reports:

log_backtrace() is thread unsafe. If threads going to assert for the same time then calling of backtrace() from every thread going to fill the same cb_buf array.

Our options here are:

  • Add a lock.
  • Make the cb_buf field non-static, and hope that the stack won't mind.

The first option looks better, I think

Child Tickets

Attachments (1)

0001-Threadproof-our-log_backtrace-implementation.patch (2.9 KB) - added by nickm 5 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 5 years ago by nickm

Oh, or there's also option 3: malloc the bytes.

comment:2 Changed 5 years ago by nickm

Status: newneeds_review

I've attached a patch and also uploaded it as branch "bug11048" to my public repo.

I bet that using a lot of stack or using malloc is probably not too bad here, since this is assertion code, not signal handler code. But I took the locking root route, since we don't want two stack traces to get jumbled up together through multiple log_err() calls. I hope I didn't mess it up. Please review?

Last edited 5 years ago by nickm (previous) (diff)

comment:3 Changed 5 years ago by cypherpunks

uploaded it as branch "bug11048"

Can't find.

Please review?

Patch seems ok.

comment:4 in reply to:  3 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Replying to cypherpunks:

uploaded it as branch "bug11048"

Can't find.

Oops; forgot to push.

Please review?

Patch seems ok.

Okay, merged it. Thanks!

Note: See TracTickets for help on using tickets.