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,

      */