Opened 5 months ago

Last modified 4 months ago

#31507 new defect

Change the client default to AvoidDiskWrites 1

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: security-low
Cc: Actual Points:
Parent ID: Points: 0.2
Reviewer: Sponsor:


Since 2007, we've seen some significant technological changes:

  • More devices with batteries
  • More devices with SSDs
  • Better disk forensics techniques

So it might be a good idea to change the client default to AvoidDiskWrites 1.

Child Tickets

Change History (3)

comment:1 Changed 5 months ago by arma

Another approach here would be to tune the "delays before writing the file to disk", so that the default is good for the default situation, and the parameter set by AvoidDiskWrites is now much bigger.

Looking through the code, here's what AvoidDiskWrites influences:

Impacting mainly relays or bridges:

  • In save_transport_to_state(), where we've picked an addr:port to listen on for our pluggable transport (when we're being a bridge I guess), we schedule our state file for a checkpoint ("or_state_mark_dirty()") but only if AvoidDiskWrites is 0.
  • In accounting_record_bandwidth_usage(), where we checkpoint our accountingmax style bandwidth usage. It's called every 10 minutes or every 20 megabytes (whichever comes first), plus also when we change hibernation state, and it schedules a flushing of the state file to disk for either "within one minute" (default) or "within two hours" (AvoidDiskWrites).
  • In hibernate_begin(), where either we began hibernation or somebody sent us a SIGINT signal to shut down, we schedule a flushing of the state file to disk for either "now" (default) or "within 10 minutes" (AvoidDiskWrites).
  • Same story (now or 10 minutes) in hibernate_go_dormant(), which happens when we hit our hard accountingmax and close all connections.
  • In rotate_onion_key(), if we just replaced our onion key, we write the new onion key to disk right then, and also we schedule a flushing of the state file to disk, for either "now" or "within an hour from now", to store the new value of state->LastRotatedOnionKey. Seems like that would be a sad one to lose, and it happens rarely, so the best fix would to uniformly pick "now".
  • In init_keys() at startup, if we just read the state file and LastRotatedOnionKey is 0 or in the past, we set it to now, and also schedule a flushing of the state file for "now" or "within an hour from now". Probably also a poor choice to wait here.

Impacting mainly clients:

  • In circuit_build_times_add_time(), where we just got another data point in how long it takes to successfully build a circuit (used in tuning the circuit build timeout feature), we call or_state_mark_dirty() every 10 circuits, but only if AvoidDiskWrites is 0.
  • Similarly in circuit_build_times_update_state(), we call or_state_mark_dirty() but only if AvoidDiskWrites is 0. This one seems redundant since we only call circuit_build_times_update_state() from or_state_save() with the comment near it "Call everything else that might dirty the state even more, in order to avoid redundant writes."
  • In entry_guards_changed_for_guard_selection(), when our entry guards list has changed, we schedule a flushing of the state file to disk for either "within 30 seconds" (default) or "within 10 minutes" (AvoidDiskWrites).
  • In entry_guards_update_state(), with the same story as circuit_build_times_update_state() above.
Last edited 5 months ago by arma (previous) (diff)

comment:2 Changed 4 months ago by nickm

Milestone: Tor: 0.4.2.x-finalTor: 0.4.3.x-final

Defer several enhancements from "new" to 0.4.3.

comment:3 Changed 4 months ago by nickm

Keywords: 042-should removed
Note: See TracTickets for help on using tickets.