Blob Blame History Raw
From 1951e7282043dfe1268d492aea056b554baedb75 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Fri, 24 Apr 2015 10:57:23 +0200
Subject: [LIBREPORT PATCH] lib: fix races in dump directory handling code

Florian Weimer <fweimer@redhat.com>:

    dd_opendir() should keep a file handle (opened with O_DIRECTORY) and
    use openat() and similar functions to access files in it.

    ...

    The file system manipulation functions should guard against hard
    links (check that link count is <= 1, just as in the user coredump
    code in abrt-hook-ccpp), possibly after opening the file
    with O_PATH first to avoid side effects on open/close.

Related: #1214745

Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
 src/include/dump_dir.h           |   7 +
 src/include/internal_libreport.h |   4 +
 src/lib/dump_dir.c               | 417 +++++++++++++++++++++++----------------
 src/lib/problem_data.c           |   6 +-
 src/lib/xfuncs.c                 |  22 ++-
 5 files changed, 275 insertions(+), 181 deletions(-)

diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index 8f672d3..d675362 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -34,6 +34,12 @@ extern "C" {
 
 /* Utility function */
 int create_symlink_lockfile(const char *filename, const char *pid_str);
+int create_symlink_lockfile_at(int dir_fd, const char *filename, const char *pid_str);
+
+/* Opens filename for reading relatively to a directory represented by dir_fd.
+ * The function fails if the file is symbolic link, directory or hard link.
+ */
+int secure_openat_read(int dir_fd, const char *filename);
 
 enum {
     DD_FAIL_QUIETLY_ENOENT = (1 << 0),
@@ -57,6 +63,7 @@ struct dump_dir {
     mode_t mode;
     time_t dd_time;
     char *dd_type;
+    int dd_fd;
 };
 
 void dd_close(struct dump_dir *dd);
diff --git a/src/include/internal_libreport.h b/src/include/internal_libreport.h
index 8d84fd4..d35d715 100644
--- a/src/include/internal_libreport.h
+++ b/src/include/internal_libreport.h
@@ -406,6 +406,8 @@ int xopen3(const char *pathname, int flags, int mode);
 int xopen(const char *pathname, int flags);
 #define xunlink libreport_xunlink
 void xunlink(const char *pathname);
+#define xunlinkat libreport_xunlinkat
+void xunlinkat(int dir_fd, const char *pathname, int flags);
 
 /* Just testing dent->d_type == DT_REG is wrong: some filesystems
  * do not report the type, they report DT_UNKNOWN for every dirent
@@ -415,6 +417,8 @@ void xunlink(const char *pathname);
  */
 #define is_regular_file libreport_is_regular_file
 int is_regular_file(struct dirent *dent, const char *dirname);
+#define is_regular_file_at libreport_is_regular_file_at
+int is_regular_file_at(struct dirent *dent, int dir_fd);
 
 #define dot_or_dotdot libreport_dot_or_dotdot
 bool dot_or_dotdot(const char *filename);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index 0048faf..16cd987 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -85,6 +85,7 @@
 
 
 static char *load_text_file(const char *path, unsigned flags);
+static char *load_text_file_at(int dir_fd, const char *name, unsigned flags);
 static void copy_file_from_chroot(struct dump_dir* dd, const char *name,
         const char *chroot_dir, const char *file_path);
 
@@ -98,10 +99,10 @@ static bool isdigit_str(const char *str)
     return true;
 }
 
-static bool exist_file_dir(const char *path)
+static bool exist_file_dir_at(int dir_fd, const char *name)
 {
     struct stat buf;
-    if (stat(path, &buf) == 0)
+    if (fstatat(dir_fd, name, &buf, AT_SYMLINK_NOFOLLOW) == 0)
     {
         if (S_ISDIR(buf.st_mode) || S_ISREG(buf.st_mode))
         {
@@ -111,15 +112,61 @@ static bool exist_file_dir(const char *path)
     return false;
 }
 
+/* Opens the file in the three following steps:
+ * 1. open the file with O_PATH (get a file descriptor for operations with
+ *    inode) and O_NOFOLLOW (do not dereference symbolick links)
+ * 2. stat the resulting file descriptor and fail if the opened file is not a
+ *    regular file or if the number of links is greater than 1 (that means that
+ *    the inode has more names (hard links))
+ * 3. "re-open" the file descriptor retrieved in the first step with O_RDONLY
+ *    by opening /proc/self/fd/$fd (then close the former file descriptor and
+ *    return the new one).
+ */
+int secure_openat_read(int dir_fd, const char *pathname)
+{
+    static char reopen_buf[sizeof("/proc/self/fd/") + 3*sizeof(int) + 1];
+
+    int path_fd = openat(dir_fd, pathname, O_PATH | O_NOFOLLOW);
+    if (path_fd < 0)
+        return -1;
+
+    struct stat path_sb;
+    int r = fstat(path_fd, &path_sb);
+    if (r < 0)
+    {
+        perror_msg("stat");
+        close(path_fd);
+        return -1;
+    }
+
+    if (!S_ISREG(path_sb.st_mode) || path_sb.st_nlink > 1)
+    {
+        log_notice("Path isn't a regular file or has more links (%lu)", path_sb.st_nlink);
+        errno = EINVAL;
+        close(path_fd);
+        return -1;
+    }
+
+    if (snprintf(reopen_buf, sizeof(reopen_buf), "/proc/self/fd/%d", path_fd) >= sizeof(reopen_buf)) {
+        error_msg("BUG: too long path to a file descriptor");
+        abort();
+    }
+
+    const int fd = open(reopen_buf, O_RDONLY);
+    close(path_fd);
+
+    return fd;
+}
+
 /* Returns value less than 0 if the file is not readable or
  * if the file doesn't contain valid unixt time stamp.
  *
  * Any possible failure will be logged.
  */
-static time_t parse_time_file(const char *filename)
+static time_t parse_time_file_at(int dir_fd, const char *filename)
 {
     /* Open input file, and parse it. */
-    int fd = open(filename, O_RDONLY | O_NOFOLLOW);
+    int fd = secure_openat_read(dir_fd, filename);
     if (fd < 0)
     {
         VERB2 pwarn_msg("Can't open '%s'", filename);
@@ -183,9 +230,9 @@ static time_t parse_time_file(const char *filename)
  *  0: failed to lock (someone else has it locked)
  *  1: success
  */
-int create_symlink_lockfile(const char* lock_file, const char* pid)
+int create_symlink_lockfile_at(int dir_fd, const char* lock_file, const char* pid)
 {
-    while (symlink(pid, lock_file) != 0)
+    while (symlinkat(pid, dir_fd, lock_file) != 0)
     {
         if (errno != EEXIST)
         {
@@ -198,7 +245,7 @@ int create_symlink_lockfile(const char* lock_file, const char* pid)
         }
 
         char pid_buf[sizeof(pid_t)*3 + 4];
-        ssize_t r = readlink(lock_file, pid_buf, sizeof(pid_buf) - 1);
+        ssize_t r = readlinkat(dir_fd, lock_file, pid_buf, sizeof(pid_buf) - 1);
         if (r < 0)
         {
             if (errno == ENOENT)
@@ -230,7 +277,7 @@ int create_symlink_lockfile(const char* lock_file, const char* pid)
             log("Lock file '%s' was locked by process %s, but it crashed?", lock_file, pid_buf);
         }
         /* The file may be deleted by now by other process. Ignore ENOENT */
-        if (unlink(lock_file) != 0 && errno != ENOENT)
+        if (unlinkat(dir_fd, lock_file, /*only files*/0) != 0 && errno != ENOENT)
         {
             perror_msg("Can't remove stale lock file '%s'", lock_file);
             errno = 0;
@@ -242,21 +289,21 @@ int create_symlink_lockfile(const char* lock_file, const char* pid)
     return 1;
 }
 
+int create_symlink_lockfile(const char *filename, const char *pid_str)
+{
+    return create_symlink_lockfile_at(AT_FDCWD, filename, pid_str);
+}
+
 static const char *dd_check(struct dump_dir *dd)
 {
-    unsigned dirname_len = strlen(dd->dd_dirname);
-    char filename_buf[FILENAME_MAX+1];
-    strcpy(filename_buf, dd->dd_dirname);
-    strcpy(filename_buf + dirname_len, "/"FILENAME_TIME);
-    dd->dd_time = parse_time_file(filename_buf);
+    dd->dd_time = parse_time_file_at(dd->dd_fd, FILENAME_TIME);
     if (dd->dd_time < 0)
     {
         log_warning("Missing file: "FILENAME_TIME);
         return FILENAME_TIME;
     }
 
-    strcpy(filename_buf + dirname_len, "/"FILENAME_TYPE);
-    dd->dd_type = load_text_file(filename_buf, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+    dd->dd_type = load_text_file_at(dd->dd_fd, FILENAME_TYPE, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
     if (!dd->dd_type || (strlen(dd->dd_type) == 0))
     {
         log_warning("Missing or empty file: "FILENAME_TYPE);
@@ -274,17 +321,12 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
     char pid_buf[sizeof(long)*3 + 2];
     snprintf(pid_buf, sizeof(pid_buf), "%lu", (long)getpid());
 
-    unsigned dirname_len = strlen(dd->dd_dirname);
-    char lock_buf[dirname_len + sizeof("/.lock")];
-    strcpy(lock_buf, dd->dd_dirname);
-    strcpy(lock_buf + dirname_len, "/.lock");
-
     unsigned count = NO_TIME_FILE_COUNT;
 
  retry:
     while (1)
     {
-        int r = create_symlink_lockfile(lock_buf, pid_buf);
+        int r = create_symlink_lockfile_at(dd->dd_fd, ".lock", pid_buf);
         if (r < 0)
             return r; /* error */
         if (r > 0)
@@ -304,8 +346,8 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
          */
         if (missing_file)
         {
-            xunlink(lock_buf);
-            log_warning("Unlocked '%s' (no or corrupted '%s' file)", lock_buf, missing_file);
+            xunlinkat(dd->dd_fd, ".lock", /*only files*/0);
+            log_warning("Unlocked '%s' (no or corrupted '%s' file)", dd->dd_dirname, missing_file);
             if (--count == 0 || flags & DD_DONT_WAIT_FOR_LOCK)
             {
                 errno = EISDIR; /* "this is an ordinary dir, not dump dir" */
@@ -326,13 +368,9 @@ static void dd_unlock(struct dump_dir *dd)
     {
         dd->locked = 0;
 
-        unsigned dirname_len = strlen(dd->dd_dirname);
-        char lock_buf[dirname_len + sizeof("/.lock")];
-        strcpy(lock_buf, dd->dd_dirname);
-        strcpy(lock_buf + dirname_len, "/.lock");
-        xunlink(lock_buf);
+        xunlinkat(dd->dd_fd, ".lock", /*only files*/0);
 
-        log_info("Unlocked '%s'", lock_buf);
+        log_info("Unlocked '%s/.lock'", dd->dd_dirname);
     }
 }
 
@@ -340,17 +378,16 @@ static inline struct dump_dir *dd_init(void)
 {
     struct dump_dir* dd = (struct dump_dir*)xzalloc(sizeof(struct dump_dir));
     dd->dd_time = -1;
+    dd->dd_fd = -1;
     return dd;
 }
 
-int dd_exist(const struct dump_dir *dd, const char *path)
+int dd_exist(const struct dump_dir *dd, const char *name)
 {
-    if (!str_is_correct_filename(path))
-        error_msg_and_die("Cannot test existence. '%s' is not a valid file name", path);
+    if (!str_is_correct_filename(name))
+        error_msg_and_die("Cannot test existence. '%s' is not a valid file name", name);
 
-    char *full_path = concat_path_file(dd->dd_dirname, path);
-    int ret = exist_file_dir(full_path);
-    free(full_path);
+    const int ret = exist_file_dir_at(dd->dd_fd, name);
     return ret;
 }
 
@@ -360,6 +397,7 @@ void dd_close(struct dump_dir *dd)
         return;
 
     dd_unlock(dd);
+    close(dd->dd_fd);
     if (dd->next_dir)
     {
         closedir(dd->next_dir);
@@ -384,10 +422,13 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
     struct dump_dir *dd = dd_init();
 
     dir = dd->dd_dirname = rm_trailing_slashes(dir);
-
+    dd->dd_fd = open(dir, O_DIRECTORY | O_NOFOLLOW);
     struct stat stat_buf;
-    if (stat(dir, &stat_buf) != 0)
+    if (dd->dd_fd < 0)
         goto cant_access;
+    if (fstat(dd->dd_fd, &stat_buf) != 0)
+        goto cant_access;
+
     /* & 0666 should remove the executable bit */
     dd->mode = (stat_buf.st_mode & 0666);
 
@@ -397,11 +438,12 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
         if ((flags & DD_OPEN_READONLY) && errno == EACCES)
         {
             /* Directory is not writable. If it seems to be readable,
-             * return "read only" dd, not NULL */
-            if (stat(dir, &stat_buf) == 0
-             && S_ISDIR(stat_buf.st_mode)
-             && access(dir, R_OK) == 0
-            ) {
+             * return "read only" dd, not NULL
+             *
+             * Does the directory have 'x' flag?
+             */
+            if (faccessat(dd->dd_fd, ".", R_OK, AT_SYMLINK_NOFOLLOW) == 0)
+            {
                 if(dd_check(dd) != NULL)
                 {
                     dd_close(dd);
@@ -444,10 +486,9 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
     if (geteuid() == 0)
     {
         /* In case caller would want to create more files, he'll need uid:gid */
-        struct stat stat_buf;
-        if (stat(dir, &stat_buf) != 0 || !S_ISDIR(stat_buf.st_mode))
+        if (fstat(dd->dd_fd, &stat_buf) != 0)
         {
-            error_msg("Can't stat '%s', or it is not a directory", dir);
+            error_msg("Can't stat '%s'", dir);
             dd_close(dd);
             return NULL;
         }
@@ -542,8 +583,7 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int
          * dd_create("dir/..") and similar are madness, refuse them.
          */
         error_msg("Bad dir name '%s'", dir);
-        dd_close(dd);
-        return NULL;
+        goto fail;
     }
 
     /* Was creating it with mode 0700 and user as the owner, but this allows
@@ -559,22 +599,31 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int
     if (r != 0)
     {
         perror_msg("Can't create directory '%s'", dir);
-        dd_close(dd);
-        return NULL;
+        goto fail;
     }
 
-    if (dd_lock(dd, CREATE_LOCK_USLEEP, /*flags:*/ 0) < 0)
+    dd->dd_fd = open(dd->dd_dirname, O_DIRECTORY | O_NOFOLLOW);
+    if (dd->dd_fd < 0)
     {
-        dd_close(dd);
-        return NULL;
+        perror_msg("Can't open newly created directory '%s'", dir);
+        goto fail;
     }
 
+    struct stat stat_sb;
+    if (fstat(dd->dd_fd, &stat_sb) < 0)
+    {
+        perror_msg("stat(%s)", dd->dd_dirname);
+        goto fail;
+    }
+
+    if (dd_lock(dd, CREATE_LOCK_USLEEP, /*flags:*/ 0) < 0)
+        goto fail;
+
     /* mkdir's mode (above) can be affected by umask, fix it */
-    if (chmod(dir, dir_mode) == -1)
+    if (fchmod(dd->dd_fd, dir_mode) == -1)
     {
         perror_msg("Can't change mode of '%s'", dir);
-        dd_close(dd);
-        return NULL;
+        goto fail;
     }
 
     dd->dd_uid = (uid_t)-1L;
@@ -616,6 +665,10 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int
     }
 
     return dd;
+
+fail:
+    dd_close(dd);
+    return NULL;
 }
 
 /* Resets ownership of the given directory to UID and GID according to values
@@ -623,7 +676,7 @@ struct dump_dir *dd_create_skeleton(const char *dir, uid_t uid, mode_t mode, int
  */
 int dd_reset_ownership(struct dump_dir *dd)
 {
-    const int r =lchown(dd->dd_dirname, dd->dd_uid, dd->dd_gid);
+    const int r = fchown(dd->dd_fd, dd->dd_uid, dd->dd_gid);
     if (r < 0)
     {
         perror_msg("Can't change '%s' ownership to %lu:%lu", dd->dd_dirname,
@@ -740,59 +793,39 @@ void dd_sanitize_mode_and_owner(struct dump_dir *dd)
     if (!dd->locked)
         error_msg_and_die("dump_dir is not opened"); /* bug */
 
-    DIR *d = opendir(dd->dd_dirname);
-    if (!d)
-        return;
-
-    struct dirent *dent;
-    while ((dent = readdir(d)) != NULL)
+    dd_init_next_file(dd);
+    char *short_name;
+    while (dd_get_next_file(dd, &short_name, /*full_name*/ NULL))
     {
-        if (dent->d_name[0] == '.') /* ".lock", ".", ".."? skip */
-            continue;
-        char *full_path = concat_path_file(dd->dd_dirname, dent->d_name);
-        struct stat statbuf;
-        if (lstat(full_path, &statbuf) == 0 && S_ISREG(statbuf.st_mode))
-        {
-            if ((statbuf.st_mode & 0777) != dd->mode)
-            {
-                /* We open the file only for fchmod()
-                 *
-                 * We use fchmod() because chmod() changes the permissions of
-                 * the file specified whose pathname is given in path, which
-                 * is dereferenced if it is a symbolic link.
-                 */
-                int fd = open(full_path, O_RDONLY | O_NOFOLLOW, dd->mode);
-                if (fd >= 0)
-                {
-                    if (fchmod(fd, dd->mode) != 0)
-                    {
-                        perror_msg("Can't change '%s' mode to 0%o", full_path,
-                                   (unsigned)dd->mode);
-                    }
-                    close(fd);
-                }
-                else
-                {
-                    perror_msg("Can't open regular file '%s'", full_path);
-                }
-            }
-            if (statbuf.st_uid != dd->dd_uid || statbuf.st_gid != dd->dd_gid)
-            {
-                if (lchown(full_path, dd->dd_uid, dd->dd_gid) != 0)
-                {
-                    perror_msg("Can't change '%s' ownership to %lu:%lu", full_path,
-                               (long)dd->dd_uid, (long)dd->dd_gid);
-                }
-            }
-        }
-        free(full_path);
+        /* The current process has to have read access at least */
+        int fd = secure_openat_read(dd->dd_fd, short_name);
+        if (fd < 0)
+            goto next;
+
+        if (fchmod(fd, dd->mode) != 0)
+            perror_msg("Can't change '%s/%s' mode to 0%o", dd->dd_dirname, short_name,
+                       (unsigned)dd->mode);
+
+        if (fchown(fd, dd->dd_uid, dd->dd_gid) != 0)
+            perror_msg("Can't change '%s/%s' ownership to %lu:%lu", dd->dd_dirname, short_name,
+                       (long)dd->dd_uid, (long)dd->dd_gid);
+
+        close(fd);
+next:
+        free(short_name);
     }
-    closedir(d);
 }
 
-static int delete_file_dir(const char *dir, bool skip_lock_file)
+static int delete_file_dir(int dir_fd, bool skip_lock_file)
 {
-    DIR *d = opendir(dir);
+    int opendir_fd = dup(dir_fd);
+    if (opendir_fd < 0)
+    {
+        perror_msg("delete_file_dir: dup(dir_fd)");
+        return -1;
+    }
+
+    DIR *d = fdopendir(opendir_fd);
     if (!d)
     {
         /* The caller expects us to error out only if the directory
@@ -818,26 +851,35 @@ static int delete_file_dir(const char *dir, bool skip_lock_file)
             unlink_lock_file = true;
             continue;
         }
-        char *full_path = concat_path_file(dir, dent->d_name);
-        if (unlink(full_path) == -1 && errno != ENOENT)
+        if (unlinkat(dir_fd, dent->d_name, /*only files*/0) == -1 && errno != ENOENT)
         {
             int err = 0;
             if (errno == EISDIR)
             {
                 errno = 0;
-                err = delete_file_dir(full_path, /*skip_lock_file:*/ false);
+                int subdir_fd = openat(dir_fd, dent->d_name, O_DIRECTORY);
+                if (subdir_fd < 0)
+                {
+                    perror_msg("Can't open sub-dir'%s'", dent->d_name);
+                    closedir(d);
+                    return -1;
+                }
+                else
+                {
+                    err = delete_file_dir(subdir_fd, /*skip_lock_file:*/ false);
+                    close(subdir_fd);
+                    if (err == 0)
+                        unlinkat(dir_fd, dent->d_name, AT_REMOVEDIR);
+                }
             }
             if (errno || err)
             {
-                perror_msg("Can't remove '%s'", full_path);
-                free(full_path);
+                perror_msg("Can't remove '%s'", dent->d_name);
                 closedir(d);
                 return -1;
             }
         }
-        free(full_path);
     }
-    closedir(d);
 
     /* Here we know for sure that all files/subdirs we found via readdir
      * were deleted successfully. If rmdir below fails, we assume someone
@@ -845,29 +887,9 @@ static int delete_file_dir(const char *dir, bool skip_lock_file)
      */
 
     if (unlink_lock_file)
-    {
-        char *full_path = concat_path_file(dir, ".lock");
-        xunlink(full_path);
-        free(full_path);
-
-        unsigned cnt = RMDIR_FAIL_COUNT;
-        do {
-            if (rmdir(dir) == 0)
-                return 0;
-            /* Someone locked the dir after unlink, but before rmdir.
-             * This "someone" must be dd_lock().
-             * It detects this (by seeing that there is no time file)
-             * and backs off at once. So we need to just retry rmdir,
-             * with minimal sleep.
-             */
-            usleep(RMDIR_FAIL_USLEEP);
-        } while (--cnt != 0);
-    }
+        xunlinkat(dir_fd, ".lock", /*only files*/0);
 
-    int r = rmdir(dir);
-    if (r)
-        perror_msg("Can't remove directory '%s'", dir);
-    return r;
+    return 0;
 }
 
 int dd_delete(struct dump_dir *dd)
@@ -878,10 +900,34 @@ int dd_delete(struct dump_dir *dd)
         return -1;
     }
 
-    int r = delete_file_dir(dd->dd_dirname, /*skip_lock_file:*/ true);
+    if (delete_file_dir(dd->dd_fd, /*skip_lock_file:*/ true) != 0)
+    {
+        perror_msg("Can't remove contents of directory '%s'", dd->dd_dirname);
+        return -2;
+    }
+
+    unsigned cnt = RMDIR_FAIL_COUNT;
+    do {
+        if (rmdir(dd->dd_dirname) == 0)
+            break;
+        /* Someone locked the dir after unlink, but before rmdir.
+         * This "someone" must be dd_lock().
+         * It detects this (by seeing that there is no time file)
+         * and backs off at once. So we need to just retry rmdir,
+         * with minimal sleep.
+         */
+        usleep(RMDIR_FAIL_USLEEP);
+    } while (--cnt != 0);
+
+    if (cnt == 0)
+    {
+        perror_msg("Can't remove directory '%s'", dd->dd_dirname);
+        return -3;
+    }
+
     dd->locked = 0; /* delete_file_dir already removed .lock */
     dd_close(dd);
-    return r;
+    return 0;
 }
 
 int dd_chown(struct dump_dir *dd, uid_t new_uid)
@@ -911,29 +957,37 @@ int dd_chown(struct dump_dir *dd, uid_t new_uid)
     gid_t groups_gid = pw->pw_gid;
 #endif
 
-    int chown_res = lchown(dd->dd_dirname, owners_uid, groups_gid);
+    int chown_res = fchown(dd->dd_fd, owners_uid, groups_gid);
     if (chown_res)
-        perror_msg("lchown('%s')", dd->dd_dirname);
+        perror_msg("fchown('%s')", dd->dd_dirname);
     else
     {
         dd_init_next_file(dd);
-        char *full_name;
-        while (chown_res == 0 && dd_get_next_file(dd, /*short_name*/ NULL, &full_name))
+        char *short_name;
+        while (chown_res == 0 && dd_get_next_file(dd, &short_name, /*full_name*/ NULL))
         {
-            log_debug("chowning %s", full_name);
-            chown_res = lchown(full_name, owners_uid, groups_gid);
+            /* The current process has to have read access at least */
+            int fd = secure_openat_read(dd->dd_fd, short_name);
+            if (fd < 0)
+                goto next;
+
+            log_debug("chowning %s", short_name);
+
+            chown_res = fchown(fd, owners_uid, groups_gid);
             if (chown_res)
-                perror_msg("lchown('%s')", full_name);
-            free(full_name);
+                perror_msg("fchownat('%s')", short_name);
+
+            close(fd);
+next:
+            free(short_name);
         }
     }
 
     return chown_res;
 }
 
-static char *load_text_file(const char *path, unsigned flags)
+static char *load_text_from_file_descriptor(int fd, const char *path, int flags)
 {
-    int fd = open(path, O_RDONLY | ((flags & DD_OPEN_FOLLOW) ? 0 : O_NOFOLLOW));
     if (fd == -1)
     {
         if (!(flags & DD_FAIL_QUIETLY_ENOENT))
@@ -988,6 +1042,20 @@ static char *load_text_file(const char *path, unsigned flags)
     return strbuf_free_nobuf(buf_content);
 }
 
+static char *load_text_file_at(int dir_fd, const char *name, unsigned flags)
+{
+    assert(name[0] != '/');
+
+    const int fd = openat(dir_fd, name, O_RDONLY | ((flags & DD_OPEN_FOLLOW) ? 0 : O_NOFOLLOW));
+    return load_text_from_file_descriptor(fd, name, flags);
+}
+
+static char *load_text_file(const char *path, unsigned flags)
+{
+    const int fd = open(path, O_RDONLY | ((flags & DD_OPEN_FOLLOW) ? 0 : O_NOFOLLOW));
+    return load_text_from_file_descriptor(fd, path, flags);
+}
+
 static void copy_file_from_chroot(struct dump_dir* dd, const char *name, const char *chroot_dir, const char *file_path)
 {
     char *chrooted_name = concat_path_file(chroot_dir, file_path);
@@ -1001,14 +1069,16 @@ static void copy_file_from_chroot(struct dump_dir* dd, const char *name, const c
     }
 }
 
-static bool save_binary_file(const char *path, const char* data, unsigned size, uid_t uid, gid_t gid, mode_t mode)
+static bool save_binary_file_at(int dir_fd, const char *name, const char* data, unsigned size, uid_t uid, gid_t gid, mode_t mode)
 {
+    assert(name[0] != '/');
+
     /* the mode is set by the caller, see dd_create() for security analysis */
-    unlink(path);
-    int fd = open(path, O_WRONLY | O_TRUNC | O_CREAT | O_NOFOLLOW, mode);
+    unlinkat(dir_fd, name, /*remove only files*/0);
+    int fd = openat(dir_fd, name, O_WRONLY | O_EXCL | O_CREAT | O_NOFOLLOW, mode);
     if (fd < 0)
     {
-        perror_msg("Can't open file '%s'", path);
+        perror_msg("Can't open file '%s'", name);
         return false;
     }
 
@@ -1016,7 +1086,9 @@ static bool save_binary_file(const char *path, const char* data, unsigned size,
     {
         if (fchown(fd, uid, gid) == -1)
         {
-            perror_msg("Can't change '%s' ownership to %lu:%lu", path, (long)uid, (long)gid);
+            perror_msg("Can't change '%s' ownership to %lu:%lu", name, (long)uid, (long)gid);
+            close(fd);
+            return false;
         }
     }
 
@@ -1028,14 +1100,16 @@ static bool save_binary_file(const char *path, const char* data, unsigned size,
      */
     if (fchmod(fd, mode) == -1)
     {
-        perror_msg("Can't change mode of '%s'", path);
+        perror_msg("Can't change mode of '%s'", name);
+        close(fd);
+        return false;
     }
 
     unsigned r = full_write(fd, data, size);
     close(fd);
     if (r != size)
     {
-        error_msg("Can't save file '%s'", path);
+        error_msg("Can't save file '%s'", name);
         return false;
     }
 
@@ -1058,11 +1132,7 @@ char* dd_load_text_ext(const struct dump_dir *dd, const char *name, unsigned fla
     if (strcmp(name, "release") == 0)
         name = FILENAME_OS_RELEASE;
 
-    char *full_path = concat_path_file(dd->dd_dirname, name);
-    char *ret = load_text_file(full_path, flags);
-    free(full_path);
-
-    return ret;
+    return load_text_file_at(dd->dd_fd, name, flags);
 }
 
 char* dd_load_text(const struct dump_dir *dd, const char *name)
@@ -1078,9 +1148,7 @@ void dd_save_text(struct dump_dir *dd, const char *name, const char *data)
     if (!str_is_correct_filename(name))
         error_msg_and_die("Cannot save text. '%s' is not a valid file name", name);
 
-    char *full_path = concat_path_file(dd->dd_dirname, name);
-    save_binary_file(full_path, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode);
-    free(full_path);
+    save_binary_file_at(dd->dd_fd, name, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode);
 }
 
 void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, unsigned size)
@@ -1091,9 +1159,7 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns
     if (!str_is_correct_filename(name))
         error_msg_and_die("Cannot save binary. '%s' is not a valid file name", name);
 
-    char *full_path = concat_path_file(dd->dd_dirname, name);
-    save_binary_file(full_path, data, size, dd->dd_uid, dd->dd_gid, dd->mode);
-    free(full_path);
+    save_binary_file_at(dd->dd_fd, name, data, size, dd->dd_uid, dd->dd_gid, dd->mode);
 }
 
 long dd_get_item_size(struct dump_dir *dd, const char *name)
@@ -1102,21 +1168,19 @@ long dd_get_item_size(struct dump_dir *dd, const char *name)
         error_msg_and_die("Cannot get item size. '%s' is not a valid file name", name);
 
     long size = -1;
-    char *iname = concat_path_file(dd->dd_dirname, name);
     struct stat statbuf;
+    int r = fstatat(dd->dd_fd, name, &statbuf, AT_SYMLINK_NOFOLLOW);
 
-    if (lstat(iname, &statbuf) == 0 && S_ISREG(statbuf.st_mode))
+    if (r == 0 && S_ISREG(statbuf.st_mode))
         size = statbuf.st_size;
     else
     {
         if (errno == ENOENT)
             size = 0;
         else
-            perror_msg("Can't get size of file '%s'", iname);
+            perror_msg("Can't get size of file '%s'", name);
     }
 
-    free(iname);
-
     return size;
 }
 
@@ -1128,18 +1192,16 @@ int dd_delete_item(struct dump_dir *dd, const char *name)
     if (!str_is_correct_filename(name))
         error_msg_and_die("Cannot delete item. '%s' is not a valid file name", name);
 
-    char *path = concat_path_file(dd->dd_dirname, name);
-    int res = unlink(path);
+    int res = unlinkat(dd->dd_fd, name, /*only files*/0);
 
     if (res < 0)
     {
         if (errno == ENOENT)
             errno = res = 0;
         else
-            perror_msg("Can't delete file '%s'", path);
+            perror_msg("Can't delete file '%s'", name);
     }
 
-    free(path);
     return res;
 }
 
@@ -1147,11 +1209,17 @@ DIR *dd_init_next_file(struct dump_dir *dd)
 {
 //    if (!dd->locked)
 //        error_msg_and_die("dump_dir is not opened"); /* bug */
+    int opendir_fd = dup(dd->dd_fd);
+    if (opendir_fd < 0)
+    {
+        perror_msg("dd_init_next_file: dup(dd_fd)");
+        return NULL;
+    }
 
     if (dd->next_dir)
         closedir(dd->next_dir);
 
-    dd->next_dir = opendir(dd->dd_dirname);
+    dd->next_dir = fdopendir(opendir_fd);
     if (!dd->next_dir)
     {
         error_msg("Can't open directory '%s'", dd->dd_dirname);
@@ -1168,7 +1236,7 @@ int dd_get_next_file(struct dump_dir *dd, char **short_name, char **full_name)
     struct dirent *dent;
     while ((dent = readdir(dd->next_dir)) != NULL)
     {
-        if (is_regular_file(dent, dd->dd_dirname))
+        if (is_regular_file_at(dent, dd->dd_fd))
         {
             if (short_name)
                 *short_name = xstrdup(dent->d_name);
@@ -1233,6 +1301,7 @@ int dd_rename(struct dump_dir *dd, const char *new_path)
         return -1;
     }
 
+    /* Keeps the opened file descriptor valid */
     int res = rename(dd->dd_dirname, new_path);
     if (res == 0)
     {
diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c
index fc07288..ebddd3c 100644
--- a/src/lib/problem_data.c
+++ b/src/lib/problem_data.c
@@ -279,14 +279,14 @@ static const char *const always_text_files[] = {
     FILENAME_OS_RELEASE,
     NULL
 };
-static char* is_text_file(const char *name, ssize_t *sz)
+static char* is_text_file_at(int dir_fd, const char *name, ssize_t *sz)
 {
     /* We were using magic.h API to check for file being text, but it thinks
      * that file containing just "0" is not text (!!)
      * So, we do it ourself.
      */
 
-    int fd = open(name, O_RDONLY);
+    int fd = secure_openat_read(dir_fd, name);
     if (fd < 0)
         return NULL; /* it's not text (because it does not exist! :) */
 
@@ -399,7 +399,7 @@ void problem_data_load_from_dump_dir(problem_data_t *problem_data, struct dump_d
         }
 
         ssize_t sz = 4*1024;
-        char *text = is_text_file(full_name, &sz);
+        char *text = is_text_file_at(dd->dd_fd, short_name, &sz);
         if (!text || text == HUGE_TEXT)
         {
             int flag = !text ? CD_FLAG_BIN : (CD_FLAG_BIN+CD_FLAG_BIGTXT);
diff --git a/src/lib/xfuncs.c b/src/lib/xfuncs.c
index 1ce44aa..979c7b8 100644
--- a/src/lib/xfuncs.c
+++ b/src/lib/xfuncs.c
@@ -331,6 +331,12 @@ int xopen(const char *pathname, int flags)
     return xopen3(pathname, flags, 0666);
 }
 
+void xunlinkat(int dir_fd, const char *pathname, int flags)
+{
+    if (unlinkat(dir_fd, pathname, flags))
+        perror_msg_and_die("Can't remove file '%s'", pathname);
+}
+
 void xunlink(const char *pathname)
 {
     if (unlink(pathname))
@@ -359,21 +365,29 @@ int open_or_warn(const char *pathname, int flags)
  * do not report the type, they report DT_UNKNOWN for every dirent
  * (and this is not a bug in filesystem, this is allowed by standards).
  */
-int is_regular_file(struct dirent *dent, const char *dirname)
+int is_regular_file_at(struct dirent *dent, int dir_fd)
 {
     if (dent->d_type == DT_REG)
         return 1;
     if (dent->d_type != DT_UNKNOWN)
         return 0;
 
-    char *fullname = xasprintf("%s/%s", dirname, dent->d_name);
     struct stat statbuf;
-    int r = lstat(fullname, &statbuf);
-    free(fullname);
+    int r = fstatat(dir_fd, dent->d_name, &statbuf, AT_SYMLINK_NOFOLLOW);
 
     return r == 0 && S_ISREG(statbuf.st_mode);
 }
 
+int is_regular_file(struct dirent *dent, const char *dirname)
+{
+    int dir_fd = open(dirname, O_DIRECTORY);
+    if (dir_fd < 0)
+        return 0;
+    int r = is_regular_file_at(dent, dir_fd);
+    close(dir_fd);
+    return r;
+}
+
 /* Is it "." or ".."? */
 /* abrtlib candidate */
 bool dot_or_dotdot(const char *filename)
-- 
1.8.3.1