andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Adding safer permissions to PosixFilesystem::NewLogger (#8106)

Browse Source 
Summary:
We have observed rocksdb databases creating info log files with world-writeable permissions.

The reason why the file is created like so is because stdio streams opened with fopen calls use mode 0666, and while normally most systems have a umask of 022, in some occasions (for instance, while running daemons), you may find that the application is running with a less restrictive umask. The result is that when opening the DB, the LOG file would be created with world-writeable perms:

```
$ ls -lh db/
total 6.4M
-rw-r--r-- 1 ibarba users  115 Mar 24 17:41 000004.log
-rw-r--r-- 1 ibarba users   16 Mar 24 17:41 CURRENT
-rw-r--r-- 1 ibarba users   37 Mar 24 17:41 IDENTITY
-rw-r--r-- 1 ibarba users    0 Mar 24 17:41 LOCK
-rw-rw-r-- 1 ibarba users 114K Mar 24 17:41 LOG
-rw-r--r-- 1 ibarba users  514 Mar 24 17:41 MANIFEST-000003
-rw-r--r-- 1 ibarba users  31K Mar 24 17:41 OPTIONS-000018
-rw-r--r-- 1 ibarba users  31K Mar 24 17:41 OPTIONS-000020
```

This diff replaces the fopen call with a regular open() call restricting mode, and then using fdopen to associate an stdio stream with that file descriptor. Resulting in the following files being created:

```
-rw-r--r-- 1 ibarba users   58 Mar 24 18:16 000004.log
-rw-r--r-- 1 ibarba users   16 Mar 24 18:16 CURRENT
-rw-r--r-- 1 ibarba users   37 Mar 24 18:16 IDENTITY
-rw-r--r-- 1 ibarba users    0 Mar 24 18:16 LOCK
-rw-r--r-- 1 ibarba users 111K Mar 24 18:16 LOG
-rw-r--r-- 1 ibarba users  514 Mar 24 18:16 MANIFEST-000003
-rw-r--r-- 1 ibarba users  31K Mar 24 18:16 OPTIONS-000018
-rw-r--r-- 1 ibarba users  31K Mar 24 18:16 OPTIONS-000020
```

With the correct permissions

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8106

Reviewed By: akankshamahajan15

Differential Revision: D27415377

Pulled By: mrambacher

fbshipit-source-id: 97ac6c215700a7ea306f4a1fdf9fcf64a3cbb202
This commit is contained in:
Imanol-Mikel Barba Sabariego 2021-03-29 20:46:18 -07:00 committed by Facebook GitHub Bot
parent a037bb35e9
commit 04191e1c5d
1 changed files with 19 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
env/fs_posix.cc vendored
View File

@ -553,24 +553,35 @@ class PosixFileSystem : public FileSystem {
IOStatus NewLogger(const std::string& fname, const IOOptions& /*opts*/,
std::shared_ptr<Logger>* result,
IODebugContext* /*dbg*/) override {
FILE* f;
FILE* f = nullptr;
int fd;
{
IOSTATS_TIMER_GUARD(open_nanos);
f = fopen(fname.c_str(),
"w"
fd = open(fname.c_str(),
cloexec_flags(O_WRONLY | O_CREAT | O_TRUNC, nullptr),
GetDBFileMode(allow_non_owner_access_));
if (fd != -1) {
f = fdopen(fd,
"w"
#ifdef __GLIBC_PREREQ
#if __GLIBC_PREREQ(2, 7)
"e" // glibc extension to enable O_CLOEXEC
"e" // glibc extension to enable O_CLOEXEC
#endif
#endif
);
);
}
}
if (f == nullptr) {
if (fd == -1) {
result->reset();
return status_to_io_status(
IOError("when fopen a file for new logger", fname, errno));
IOError("when open a file for new logger", fname, errno));
}
if (f == nullptr) {
close(fd);
result->reset();
return status_to_io_status(
IOError("when fdopen a file for new logger", fname, errno));
} else {
int fd = fileno(f);
#ifdef ROCKSDB_FALLOCATE_PRESENT
fallocate(fd, FALLOC_FL_KEEP_SIZE, 0, 4 * 1024);
#endif