tor should remove empty/invalid consensus cache files (Unable to map file from consensus cache: Numerical result out of range)

I use XFS, which tends to leave newly-written files empty on crash rather than non-existent. tor should handle this better.

changed milestone to %Tor: 0.3.1.x-final

added 031-backport component::core tor/tor milestone::Tor: 0.3.1.x-final owner::nickm priority::low resolution::fixed reviewer::ahf severity::minor status::closed type::defect labels

Trac:
Milestone: N/A to Tor: 0.3.2.x-final

Trac:
Status: new to accepted
Owner: N/A to nickm

Branch bug24099_031 tries to fix this. Please review?

Trac:
Keywords: N/A deleted, 031-backport added
Status: accepted to needs_review

I'm a bit confused here, ERANGE is not documented as an errno value from mmap(2) both on Linux and FreeBSD. It seems that the range issue is handled with:

  EINVAL We don't like addr, length, or offset (e.g., they are too large, or not aligned on a page boundary).

@Hello71, what is your kernel version here? I've checked (on kernel 4.14) and XFS doesn't seem to be returning ERANGE in the case of a mmap...

The fix lgtm; but this ERANGE thing is just weird and confusing to me from which syscall this can come. The open(), fstat() and mmap() don't seem to never return that :S ...

Trac:
Reviewer: N/A to ahf

/** Try to create a memory mapping for <b>filename</b> and return it.  On
 * failure, return NULL.  Sets errno properly, using ERANGE to mean
 * "empty file". */
tor_mmap_t *
tor_mmap_file(const char *filename)

Oh true, it is "artificial" there. Fair enough, patch lgtm;

cabcb752d7ecc2d16e6cb630b3de0684b4f97ec5 looks good.

As per discussion on IRC, in c8ee12b2e8108658d647aedb92885311291b6f71, it would be useful to have a comment in consensus_cache_rescan() about the meaning of errno == ERANGE after calling tor_mmap_file().

I think we are generally a bit inconsistent about this, but there is an empty line at the end of changes/bug24099, but it looks like that shouldn't really matter.

Otherwise the patches look good.

Trac:
Status: needs_review to merge_ready

I've added an explanatory comment to the branch, and am now about to merge to 0.3.2 and forward.

It's okay to be inconsistent about blanks at the end of changes files: our tools clean them up when the changelog is built.

Merged to 0.3.2 and forward; marking for possible backport.

Trac:
Milestone: Tor: 0.3.2.x-final to Tor: 0.3.1.x-final

merged to 0.3.1!

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

moved to tpo/core/tor#24099 (closed)