boostorg / boostorg/iostreams

Double close possible in file_descriptor_impl::close_impl()

Open
#143 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
48
Forks
124
PR merge metrics
No merged PRs in 30d

Description

The current implementation of file_descriptor_impl::close_impl() will cause a double close on a file descriptor if you explicitly call close() and it results in an error (e.g. BOOST_IOSTREAMS_FD_CLOSE() returns a failure due to EIO or ENOSPC). In this case, the first call to close_impl() (from file_descriptor_impl::close()) will call the operating system's close() call on handle_ then throw an exception before it clears handle_ (by setting it to invalid_handle()). When the destructor is called, handle_ is still set to the old file handle, so the destructor will call the operating system's close() again. In the mean time, another thread may have opened that same file / socket handle. If you're lucky, that other thread will get some kind of IO error. If you're not lucky (as was my case), yet another thread then opens the same file handle (because it was closed again), at which point you have really weird file corruption.

Proposed patch:
diff --git a/src/file_descriptor.cpp b/src/file_descriptor.cpp
index 288e2c6..77dd65d 100644
```
--- a/src/file_descriptor.cpp
+++ b/src/file_descriptor.cpp
@@ -266,8 +266,11 @@ void file_descriptor_impl::close_impl(bool close_flag, bool throw_) {
#else
BOOST_IOSTREAMS_FD_CLOSE(handle_) != -1;
#endif
- if (!success && throw_)
+ if (!success && throw_) {
+ handle_ = invalid_handle();
+ flags_ = 0;
throw_system_failure("failed closing file");
+ }
}
handle_ = invalid_handle();
flags_ = 0;
```

I'm not sure about flags_, but handle should definitely be set to invalid_handle() before throwing the exception.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/file_descriptor.cpp at file_descriptor_impl::close_impl(), then trace the explicit close() path and destructor cleanup. Verify the failure path after BOOST_IOSTREAMS_FD_CLOSE() does not leave the handle available for a second close, and check whether flags_ should be cleared as well; done means the proposed regression is prevented without changing successful cleanup.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.