Opened 5 years ago

Closed 4 years ago

#16274 closed defect (fixed)

Set the max open file value to the current limit if setrlimit() fails

Reported by: dgoulet Owned by:
Priority: Very High Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: TorCoreTeam201508
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In set_max_file_descriptors(), if setrlimit() fails, we log warn and bail out leaving the max_out pointer unset.

It should be set to the current value returned by getrlimit() before we try to change it with setrlimit().

Flagging this as critical because right now on commit e48f8e5e87603812a6b1844a5fa27bbc44a3543e, if the ed25519_master_id_public_key can't be opened, tor shuts down.

Child Tickets

Change History (8)

comment:1 Changed 5 years ago by dgoulet

Status: newneeds_review

Does that look like a valid fix? :)

Branch: bug16274_027_01

comment:2 Changed 5 years ago by nickm

It seems simple enough, but the documentation for the function needs to change. Also, since this is a behavior change, I'm a bit unsure that the callers of set_max_file_descriptors() actually respect this value correctly. What do you think?

comment:3 Changed 4 years ago by nickm

Keywords: TorCoreTeam201507 added

comment:4 Changed 4 years ago by nickm

Status: needs_reviewneeds_revision

comment:5 Changed 4 years ago by nickm

Keywords: TorCoreTeam201508 added; TorCoreTeam201507 removed

comment:6 Changed 4 years ago by dgoulet

Keywords: TorCoreTeam201507 added; TorCoreTeam201508 removed

comment:7 Changed 4 years ago by dgoulet

Keywords: TorCoreTeam201508 added; TorCoreTeam201507 removed
Status: needs_revisionneeds_review

New branch with updated documentation for set_max_file_descriptors(). Two places call this function in config.c, one of them checks the returned value and the second one is the rollback to the previous value which doesn't check the return value but seems there is no clean way to fail anyway there. So I think we are OK with this branch.

See branch: bug16274_027_02

comment:8 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Great, merged!

Note: See TracTickets for help on using tickets.