Commit 8ce7ef50 authored by Ramon Nou's avatar Ramon Nou
Browse files

Implement fd protection mechanism and update documentation

parent d3517cd3
Loading
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -9,6 +9,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
### New
  - Added cppcheck code checking capabilities ([!214](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/214))
  - Tests to cover proxy and malleability ([!222](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/222))
  - New fd generation method ([!225](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/202))
    - Use LIBGKFS_PROTECT_FD=1 to enable the original method of assignation and protection.
    
### Changed 
  - Tests check ret for -1 instead of 10000 fd ([!320](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/320))
+4 −0
Original line number Diff line number Diff line
@@ -585,6 +585,10 @@ until the file is closed. The cache does not impact the consistency of the file
  with the corresponding daemon (default: 1000). The file size is further synchronized when the file is `close()`d or
  when `fsync()` is called.

##### Protecting FDs
When the user creates a fd, this is protected from normal fds with a recolocation. This theoretically protects the fd from being closed from outside. However a new fd assignation system has been developed and is activated by default.
- `LIBGKFS_PROTECT_FD=1` - Enable the original method of assignation and protection.

### Daemon
#### Logging
- `GKFS_DAEMON_LOG_PATH` - Path to the log file of the daemon.
+1 −0
Original line number Diff line number Diff line
@@ -68,6 +68,7 @@ static constexpr auto METRICS_PATH = ADD_PREFIX("METRICS_PATH");
static constexpr auto METRICS_IP_PORT = ADD_PREFIX("METRICS_IP_PORT");
#endif

static constexpr auto PROTECT_FD = ADD_PREFIX("PROTECT_FD");
static constexpr auto NUM_REPL = ADD_PREFIX("NUM_REPL");
static constexpr auto PROXY_PID_FILE = ADD_PREFIX("PROXY_PID_FILE");
namespace cache {
+7 −0
Original line number Diff line number Diff line
@@ -138,6 +138,7 @@ private:
    std::bitset<MAX_USER_FDS> protected_fds_;
    std::string hostname;
    int replicas_;
    bool protect_fds_{false};

    std::shared_ptr<gkfs::messagepack::ClientMetrics> write_metrics_;
    std::shared_ptr<gkfs::messagepack::ClientMetrics> read_metrics_;
@@ -314,6 +315,12 @@ public:
    int
    get_replicas();

    bool
    protect_fds() const;

    void
    protect_fds(bool protect);

    const std::shared_ptr<gkfs::messagepack::ClientMetrics>
    write_metrics();

+27 −16
Original line number Diff line number Diff line
@@ -142,23 +142,30 @@ OpenFileMap::exist(const int fd) {

int
OpenFileMap::safe_generate_fd_idx_() {
    auto fd = generate_fd_idx();
    int fd = 0;
    if(CTX->protect_fds()) {
        fd = generate_fd_idx();
        /*
         * Check if fd is still in use and generate another if yes
     * Note that this can only happen once the all fd indices within the int has
     * been used to the int::max Once this limit is exceeded, we set fd_idx back
     * to 3 and begin anew. Only then, if a file was open for a long time will
     * we have to generate another index.
         * Note that this can only happen once the all fd indices within the int
         * has been used to the int::max Once this limit is exceeded, we set
         * fd_idx back to 3 and begin anew. Only then, if a file was open for a
         * long time will we have to generate another index.
         *
     * This situation can only occur when all fd indices have been given away
     * once and we start again, in which case the fd_validation_needed flag is
     * set. fd_validation is set to false, if
         * This situation can only occur when all fd indices have been given
         * away once and we start again, in which case the fd_validation_needed
         * flag is set. fd_validation is set to false, if
         */
        if(fd_validation_needed) {
            while(exist(fd)) {
                fd = generate_fd_idx();
            }
        }
    } else {
        // Some architectures do not support SYS_open
        fd = syscall_no_intercept(SYS_openat, AT_FDCWD, "/dev/null", O_RDWR,
                                  S_IRUSR | S_IWUSR);
    }
    return fd;
}

@@ -178,6 +185,10 @@ OpenFileMap::remove(const int fd) {
        return false;
    }
    files_.erase(fd);
    if(!CTX->protect_fds()) {
        // We close the dev null fd
        close(fd);
    }
    if(fd_validation_needed && files_.empty()) {
        fd_validation_needed = false;
        LOG(DEBUG, "fd_validation flag reset");
@@ -237,7 +248,7 @@ OpenFileMap::generate_fd_idx() {
         * which tells can tell the OpenFileMap that it should check if this fd
         * is really safe to use.
         */
        fd_idx = 100000;
        fd_idx = 10000;
        fd_validation_needed = true;
    }
    return fd_idx++;
Loading