Opened 7 months ago

Closed 6 weeks ago

#29617 closed defect (fixed)

OOM manger wipes entire DNS cache

Reported by: pulls Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.4.0.2-alpha
Severity: Normal Keywords: 035-backport, 040-backport, fast-fix, easy, dgoulet-merge
Cc: Actual Points: 0
Parent ID: Points: 0
Reviewer: ahf Sponsor:

Description

In relay.c, function cell_queues_check_size, the OOM manager attempts to clear one tenth of MaxMemInQueues bytes from the DNS cache by calling dns_cache_handle_oom. The function dns_cache_handle_oom, in dns.c, runs in a loop removing cached entries that are now+n*time_inc old, until at least the requested number of bytes have been freed. The first iteration of the loop has n=0, and likely will not remove enough bytes. The second iteration is way too aggressive, because:

time_inc += 3600; /* Increase time_inc by 1 hour. */

This is guaranteed to wipe the entire DNS cache, because in dns_clip_ttl the maximum time to cache is MAX_DNS_TTL_AT_EXIT, which is set in dns.h to:

/** Lowest value for DNS ttl that a server will give. */
#define MIN_DNS_TTL_AT_EXIT (5*60)
/** Highest value for DNS ttl that a server will give. */
#define MAX_DNS_TTL_AT_EXIT (60*60)

One possible and reasonable fix would be to instead increment time_inc by MIN_DNS_TTL_AT_EXIT.

Child Tickets

Change History (11)

comment:1 Changed 7 months ago by pulls

Component: - Select a componentCore Tor/Tor
Version: Tor: 0.4.0.2-alpha

comment:2 Changed 7 months ago by nickm

Keywords: 035-backport 040-backport fast-fix easy added
Milestone: Tor: 0.4.1.x-final
Points: 0

comment:3 Changed 4 months ago by nickm

Keywords: 041-should added

comment:4 Changed 4 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:5 Changed 4 months ago by nickm

Actual Points: 0
Status: acceptedneeds_review

See branch bug29617_035 with PR at https://github.com/torproject/tor/pull/1034

comment:6 Changed 4 months ago by asn

Reviewer: ahf

comment:7 Changed 4 months ago by ahf

Status: needs_reviewmerge_ready

Looks good to me.

comment:8 Changed 4 months ago by nickm

Keywords: dgoulet-merge added

comment:9 Changed 4 months ago by dgoulet

Milestone: Tor: 0.4.1.x-finalTor: 0.4.0.x-final

Merged in 041! Moving to 040 for backport.

comment:10 Changed 4 months ago by nickm

Keywords: 041-should removed

Since this is merged in 041, it is no longer 041-should.

comment:11 Changed 6 weeks ago by teor

Milestone: Tor: 0.4.0.x-finalTor: 0.3.5.x-final
Resolution: fixed
Status: merge_readyclosed

Backported to 0.3.5.
Merged with the other 0.3.5 and 0.4.0 backports on 2019-08-12.

Note: See TracTickets for help on using tickets.