Opened 5 weeks ago

Closed 5 weeks ago

Last modified 5 weeks ago

#22370 closed defect (fixed)

Stop leaking routerinfos that are rejected due to keypinning

Reported by: teor Owned by:
Priority: High Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7.2-alpha
Severity: Major Keywords: memory-leak, 029-backport, 030-backport
Cc: Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: Sponsor:

Description

When we added the keypinning code to dirserv_add_descriptor, we forgot to free rejected routerinfos.

We should backport this to 0.2.9, because it is the earliest version run by the public directory authorities. (And it's LTS.)

Child Tickets

Change History (8)

comment:1 Changed 5 weeks ago by teor

  • Priority changed from Medium to High
  • Severity changed from Normal to Major
  • Status changed from new to needs_review

Please see my branch bug22370-029 on github.
Key pinning was turned on by default in 0.3.0.2-alpha.
3/8 directory authorities are running versions > 0.3.0.2-alpha.

comment:2 Changed 5 weeks ago by teor

Opened #22372 to try to make sure we don't do this again.

comment:3 follow-up: Changed 5 weeks ago by arma

Looks like the cause of #21926 perhaps?

comment:4 Changed 5 weeks ago by arma

#22349 probably makes this way worse. :)

comment:5 Changed 5 weeks ago by arma

I made a bug22370-029-2 branch that fixes a typo in your commit message, removes a "tor-" from the changes file, and expands the function comment a bit. I'm afraid it's rebased since I don't know how to change commit messages otherwise. Do you like it?

comment:6 Changed 5 weeks ago by teor

  • Status changed from needs_review to merge_ready

Looks fine to me.
Remember we want to backport this to 0.2.9 and later.
But we don't have to do that right now.

comment:7 Changed 5 weeks ago by arma

  • Resolution set to fixed
  • Status changed from merge_ready to closed

I have merged, to 0.2.9 and up.

comment:8 in reply to: ↑ 3 Changed 5 weeks ago by arma

Replying to arma:

Looks like the cause of #21926 perhaps?

Yes! I think #21926 is resolved now.

Note: See TracTickets for help on using tickets.