[BUG][CRASH] Occasional crashes when reloading file mounted with SSHFS

[kak-0]

Version of Kakoune

v2021.11.08-59-g9acd4e62

Reproducer

Kakoune occasionally crashes when reloading a file that's mounted with SSHFS after the file is modified outside of Kakoune (e.g. after running git checkout on the remote machine).

Outcome

Killed by signal SIGBUS.

Backtrace:

#0  0x000055d76d10dc99 parse_file<Kakoune::reload_file_buffer(Kakoune::Buffer&)::<lambda(auto:39&& ...)> > (kak + 0xc1c99)
#1  0x000055d76d10e0e1 Kakoune::reload_file_buffer(Kakoune::Buffer&) (kak + 0xc20e1)
#2  0x000055d76d11184d Kakoune::Client::reload_buffer() (kak + 0xc584d)
#3  0x000055d76d116560 Kakoune::Client::on_buffer_reload_key(Kakoune::Key) (kak + 0xca560)
#4  0x000055d76d1c812e std::function<void (Kakoune::Key, Kakoune::Context&)>::operator()(Kakoune::Key, Kakoune::Context&) const (kak + 0x17c12e)
#5  0x000055d76d1c3347 Kakoune::InputMode::handle_key(Kakoune::Key) (kak + 0x177347)
#6  0x000055d76d113f2c Kakoune::Client::process_pending_inputs() (kak + 0xc7f2c)
#7  0x000055d76d11bcc8 Kakoune::ClientManager::process_pending_inputs() (kak + 0xcfcc8)
#8  0x000055d76d2126fc Kakoune::run_server(Kakoune::StringView, Kakoune::StringView, Kakoune::StringView, Kakoune::Optional<Kakoune::BufferCoord>, Kakoune::ServerFlags, Kakoune::UIType, Kakoune::DebugFlags, Kakoune::ArrayView<Kakoune::StringView const>) (kak + 0x1c66fc)
#9  0x000055d76d0f78d9 main (kak + 0xab8d9)
#10 0x00007fb655299b25 __libc_start_main (libc.so.6 + 0x27b25)
#11 0x000055d76d0f82de _start (kak + 0xac2de)

Expectations

No response

Additional information

I got a SIGBUS signal in parse_file on this line:

        if (*it == '\n')

This line tries to read a memory-mapped file mapped in MappedFile::MappedFile:

    data = (const char*)mmap(nullptr, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);

mmaps(2) says "It is unspecified whether changes made to the file after the mmap() call are visible in the mapped region" when using MAP_PRIVATE. Maybe the crash is due to a race condition where the file contents or size changes while Kakoune is reading it.

This could also be a bug in how SSHFS implements mmap.

In either case, I guess the only workaround is to read the file with read(2) instead of mmap(2).

Does anyone else experience crashes when using SSHFS? Is there another workaround?

[krobelus]

The SIGBUS also happens to kak file -e 'set global autoreload yes' when running while true; do echo $RANDOM > file; done, so I don't think this is specific to SSHFS.

[Screwtapello]

mmap(2) says:

SIGBUS: Attempted access to a page of the buffer that lies beyond the end of the mapped file.

The parse_file() function pointed to is, I think, this one:

kakoune/src/buffer_utils.cc

Lines 119 to 145 in 0b29fcf

	template<typename Func>
	decltype(auto) parse_file(StringView filename, Func&& func)
	{
	MappedFile file{parse_filename(filename)};
	const char* pos = file.data;
	const char* end = pos + file.st.st_size;
	auto bom = ByteOrderMark::None;
	if (file.st.st_size >= 3 && StringView{pos, 3_byte} == "\xEF\xBB\xBF")
	{
	bom = ByteOrderMark::Utf8;
	pos += 3;
	}
	bool has_crlf = false, has_lf = false;
	for (auto it = pos; it != end; ++it)
	{
	if (*it == '\n')
	((it != pos and *(it-1) == '\r') ? has_crlf : has_lf) = true;
	}
	const bool crlf = has_crlf and not has_lf;
	auto eolformat = crlf ? EolFormat::Crlf : EolFormat::Lf;
	FsStatus fs_status{file.st.st_mtim, file.st.st_size, hash_data(file.data, file.st.st_size)};
	return func(parse_lines(pos, end, eolformat), bom, eolformat, fs_status);
	}

Note that we get the file size on line 122, and then access the file data on line 137, a classic time-of-check-to-time-of-use (TOCTOU) bug. It's possible (although unlikely) for the file-size to change between those two moments (as in krobelus' test-case) and cause Kakoune to crash.

It's possible that SSHFS makes the problem worse - maybe it caches the expected file-size locally (so that the 'time of check' is not when Kakoune checks, but when the cache was last refreshed), or maybe it's just network latency that pushes "time-of-check" and "time-of-use" far enough apart to cause problems.

Unfortunately, unlike other APIs there's no good way to mmap() the entire file in one step and prevent TOCTOU bugs entirely. Alternatives include:

• run Kakoune on the remote host via SSH, minimising Kakoune<->disk latency at the expense of user<->Kakoune latency
• run the Kakoune session on the remote host, forward the session socket to the local machine with SSH, and run a local client
• add an option to Kakoune to disable mmap(2) and only use read(2)/write(2) for file I/O.