Skip to content

Commit bbc2585

Browse files
committed
tac: read regular files instead of mmap to avoid SIGBUS on truncation
tac memory-mapped regular files. If another process truncated such a file while it was mapped (e.g. during log rotation), accessing the now-invalid pages raised SIGBUS and killed the process. Read regular files into memory up front instead, so a concurrent truncation can no longer crash tac. stdin still uses mmap on its process-owned temp file, where external truncation is not a concern. Adds a regression test that truncates a file mid-read and asserts tac is not killed by a signal. Fixes #9748
1 parent b21e227 commit bbc2585

2 files changed

Lines changed: 61 additions & 22 deletions

File tree

src/uu/tac/src/tac.rs

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -383,20 +383,21 @@ fn tac(filenames: &[OsString], before: bool, regex: bool, separator: &OsStr) ->
383383
}
384384
};
385385

386-
if let Some(mmap1) = try_mmap_file(&file) {
387-
mmap = mmap1;
388-
&mmap
389-
} else {
390-
let mut contents = Vec::new();
391-
match file.read_to_end(&mut contents) {
392-
Ok(_) => {
393-
buf = contents;
394-
&buf
395-
}
396-
Err(e) => {
397-
show!(TacError::ReadError(filename.clone(), e));
398-
continue;
399-
}
386+
// Read the whole file into memory instead of memory-mapping it.
387+
// Memory-mapping a regular file is unsound: if another process
388+
// truncates the file while it is mapped, accessing the now-invalid
389+
// pages raises SIGBUS and terminates the process (for example
390+
// during log rotation). Copying the bytes up front avoids this.
391+
// See https://github.com/uutils/coreutils/issues/9748.
392+
let mut contents = Vec::new();
393+
match file.read_to_end(&mut contents) {
394+
Ok(_) => {
395+
buf = contents;
396+
&buf
397+
}
398+
Err(e) => {
399+
show!(TacError::ReadError(filename.clone(), e));
400+
continue;
400401
}
401402
}
402403
};
@@ -450,12 +451,6 @@ fn buffer_stdin() -> std::io::Result<StdinData> {
450451
}
451452
}
452453

453-
fn try_mmap_file(file: &File) -> Option<Mmap> {
454-
// SAFETY: If the file is truncated while we map it, SIGBUS will be raised
455-
// and our process will be terminated, thus preventing access of invalid memory.
456-
unsafe { Mmap::map(file).ok() }
457-
}
458-
459454
#[cfg(test)]
460455
mod tests_hybrid_flavor {
461456
use super::translate_regex_flavor;

tests/by-util/test_tac.rs

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
//
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
5-
// spell-checker:ignore axxbxx bxxaxx axxx axxxx xxaxx xxax xxxxa axyz zyax zyxa bbaaa aaabc bcdddd cddddaaabc xyzabc abcxyzabc nbbaaa EISDIR
6-
#[cfg(target_os = "linux")]
5+
// spell-checker:ignore axxbxx bxxaxx axxx axxxx xxaxx xxax xxxxa axyz zyax zyxa bbaaa aaabc bcdddd cddddaaabc xyzabc abcxyzabc nbbaaa EISDIR SIGBUS mmap
6+
#[cfg(unix)]
77
use uutests::at_and_ucmd;
88
use uutests::new_ucmd;
99
use uutests::util::TestScenario;
@@ -475,3 +475,47 @@ fn test_regular_end_anchor() {
475475
.succeeds()
476476
.stdout_is("\nccc\nbbaaa\nb");
477477
}
478+
479+
/// Regression test for <https://github.com/uutils/coreutils/issues/9748>.
480+
///
481+
/// `tac` used to memory-map regular files. When another process truncated such
482+
/// a file while it was mapped, accessing the now-invalid pages raised SIGBUS
483+
/// and killed the process. Now that `tac` reads files into memory up front, a
484+
/// concurrent truncation can no longer crash it: it reverses whatever it
485+
/// managed to read (or reports a normal I/O error).
486+
///
487+
/// On a build still using mmap this race would intermittently terminate the
488+
/// process with SIGBUS; on the fixed build it can never be killed by a signal,
489+
/// so this assertion is stable regardless of timing.
490+
#[test]
491+
#[cfg(unix)]
492+
fn test_tac_file_truncated_during_read_does_not_crash() {
493+
use std::fs::OpenOptions;
494+
use std::time::Duration;
495+
496+
let (at, mut ucmd) = at_and_ucmd!();
497+
498+
// A large sparse file, so the read overlaps with the truncation below.
499+
// `set_len` keeps it sparse, so creation stays cheap.
500+
let name = "input";
501+
at.make_file(name).set_len(64 * 1024 * 1024).unwrap();
502+
503+
let child = ucmd.arg(name).run_no_wait();
504+
505+
// Give tac a moment to start reading, then truncate the file out from
506+
// under it.
507+
std::thread::sleep(Duration::from_millis(2));
508+
OpenOptions::new()
509+
.write(true)
510+
.open(at.plus(name))
511+
.unwrap()
512+
.set_len(0)
513+
.unwrap();
514+
515+
let result = child.wait().unwrap();
516+
assert!(
517+
result.signal().is_none(),
518+
"tac was killed by signal {:?} (SIGBUS regression, see #9748)",
519+
result.signal()
520+
);
521+
}

0 commit comments

Comments
 (0)