From 77ab42619d964c64cb33437511c931cd71db3d47 Mon Sep 17 00:00:00 2001 From: Marc Vef Date: Mon, 2 Mar 2020 15:27:40 +0100 Subject: [PATCH] Fixing alignment bugs in getdents(), adding comments --- include/client/gkfs_functions.hpp | 8 +-- include/client/hooks.hpp | 8 +-- include/client/open_dir.hpp | 2 - src/client/gkfs_functions.cpp | 82 +++++++++++++++++++++++-------- src/client/hooks.cpp | 5 +- src/client/intercept.cpp | 4 +- 6 files changed, 74 insertions(+), 35 deletions(-) diff --git a/include/client/gkfs_functions.hpp b/include/client/gkfs_functions.hpp index 328a64a18..7ae71ce99 100644 --- a/include/client/gkfs_functions.hpp +++ b/include/client/gkfs_functions.hpp @@ -19,8 +19,8 @@ struct statfs; struct statvfs; -struct dirent; -struct dirent64; +struct linux_dirent; +struct linux_dirent64; namespace gkfs { namespace syscall { @@ -79,9 +79,9 @@ ssize_t gkfs_read(int fd, void* buf, size_t count); int gkfs_opendir(const std::string& path); -int gkfs_getdents(unsigned int fd, struct dirent* dirp, unsigned int count); +int gkfs_getdents(unsigned int fd, struct linux_dirent* dirp, unsigned int count); -int gkfs_getdents64(unsigned int fd, struct dirent64* dirp, unsigned int count); +int gkfs_getdents64(unsigned int fd, struct linux_dirent64* dirp, unsigned int count); int gkfs_rmdir(const std::string& path); diff --git a/include/client/hooks.hpp b/include/client/hooks.hpp index 3c639d036..a0aacee42 100644 --- a/include/client/hooks.hpp +++ b/include/client/hooks.hpp @@ -18,8 +18,8 @@ #include struct statfs; -struct dirent; -struct dirent64; +struct linux_dirent; +struct linux_dirent64; namespace gkfs { namespace hook { @@ -69,9 +69,9 @@ int hook_dup2(unsigned int oldfd, unsigned int newfd); int hook_dup3(unsigned int oldfd, unsigned int newfd, int flags); -int hook_getdents(unsigned int fd, struct dirent* dirp, unsigned int count); +int hook_getdents(unsigned int fd, struct linux_dirent* dirp, unsigned int count); -int hook_getdents64(unsigned int fd, struct dirent64* dirp, unsigned int count); +int hook_getdents64(unsigned int fd, struct linux_dirent64* dirp, unsigned int count); int hook_mkdirat(int dirfd, const char* cpath, mode_t mode); diff --git a/include/client/open_dir.hpp b/include/client/open_dir.hpp index 46fbc796d..e8111631b 100644 --- a/include/client/open_dir.hpp +++ b/include/client/open_dir.hpp @@ -18,8 +18,6 @@ #include #include -#include - #include namespace gkfs { diff --git a/src/client/gkfs_functions.cpp b/src/client/gkfs_functions.cpp index 64dc15555..029304a77 100644 --- a/src/client/gkfs_functions.cpp +++ b/src/client/gkfs_functions.cpp @@ -24,16 +24,41 @@ #include extern "C" { +#include // used for file types in the getdents{,64}() functions +#include // used for definition of alignment macros #include #include -#include } -#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +using namespace std; + +/* + * Macro used within getdents{,64} functions. + * __ALIGN_KERNEL defined in linux/kernel.h + */ #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) -using namespace std; +/* + * linux_dirent is used in getdents() but is privately defined in the linux kernel: fs/readdir.c. + */ +struct linux_dirent { + unsigned long d_ino; + unsigned long d_off; + unsigned short d_reclen; + char d_name[1]; +}; +/* + * linux_dirent64 is used in getdents64() and defined in the linux kernel: include/linux/dirent.h. + * However, it is not part of the kernel-headers and cannot be imported. + */ +struct linux_dirent64 { + uint64_t d_ino; + int64_t d_off; + unsigned short d_reclen; + unsigned char d_type; + char d_name[1]; // originally `char d_name[0]` in kernel, but ISO C++ forbids zero-size array 'd_name' +}; + namespace { @@ -505,9 +530,10 @@ int gkfs_rmdir(const std::string& path) { } int gkfs_getdents(unsigned int fd, - struct dirent* dirp, + struct linux_dirent* dirp, unsigned int count) { + // Get opendir object (content was downloaded with opendir() call) auto open_dir = CTX->file_map()->get_dir(fd); if (open_dir == nullptr) { //Cast did not succeeded: open_file is a regular file @@ -515,24 +541,32 @@ int gkfs_getdents(unsigned int fd, return -1; } + // get directory position of which entries to return auto pos = open_dir->pos(); if (pos >= open_dir->size()) { return 0; } unsigned int written = 0; - struct dirent* current_dirp = nullptr; + struct linux_dirent* current_dirp = nullptr; while (pos < open_dir->size()) { - gkfs::filemap::DirEntry de = open_dir->getdent(pos); + // get dentry fir current position + auto de = open_dir->getdent(pos); + /* + * Calculate the total dentry size within the kernel struct `linux_dirent` depending on the file name size. + * The size is then aligned to the size of `long` boundary. + * This line was originally defined in the linux kernel: fs/readdir.c in function filldir(): + * int reclen = ALIGN(offsetof(struct linux_dirent, d_name) + namlen + 2, sizeof(long)); + * However, since d_name is null-terminated and de.name().size() does not include space + * for the null-terminator, we add 1. Thus, + 3 in total. + */ auto total_size = ALIGN(offsetof( - struct dirent, d_name) + - de.name().size() + 3, sizeof(long)); + struct linux_dirent, d_name) + de.name().size() + 3, sizeof(long)); if (total_size > (count - written)) { //no enough space left on user buffer to insert next dirent break; } - current_dirp = reinterpret_cast( - reinterpret_cast(dirp) + written); + current_dirp = reinterpret_cast(reinterpret_cast(dirp) + written); current_dirp->d_ino = std::hash()( open_dir->path() + "/" + de.name()); @@ -552,13 +586,14 @@ int gkfs_getdents(unsigned int fd, errno = EINVAL; return -1; } + // set directory position for next getdents() call open_dir->pos(pos); return written; } int gkfs_getdents64(unsigned int fd, - struct dirent64* dirp, + struct linux_dirent64* dirp, unsigned int count) { auto open_dir = CTX->file_map()->get_dir(fd); @@ -567,25 +602,32 @@ int gkfs_getdents64(unsigned int fd, errno = EBADF; return -1; } - auto pos = open_dir->pos(); if (pos >= open_dir->size()) { return 0; } - unsigned int written = 0; - struct dirent64* current_dirp = nullptr; + struct linux_dirent64* current_dirp = nullptr; while (pos < open_dir->size()) { - gkfs::filemap::DirEntry de = open_dir->getdent(pos); + auto de = open_dir->getdent(pos); + /* + * Calculate the total dentry size within the kernel struct `linux_dirent` depending on the file name size. + * The size is then aligned to the size of `long` boundary. + * + * This line was originally defined in the linux kernel: fs/readdir.c in function filldir64(): + * int reclen = ALIGN(offsetof(struct linux_dirent64, d_name) + namlen + 1, sizeof(u64)); + * We keep + 1 because: + * Since d_name is null-terminated and de.name().size() does not include space + * for the null-terminator, we add 1. Since d_name in our `struct linux_dirent64` definition + * is not a zero-size array (as opposed to the kernel version), we subtract 1. Thus, it stays + 1. + */ auto total_size = ALIGN(offsetof( - struct dirent64, d_name) + - de.name().size() + 3, sizeof(long)); + struct linux_dirent64, d_name) + de.name().size() + 1, sizeof(uint64_t)); if (total_size > (count - written)) { //no enough space left on user buffer to insert next dirent break; } - current_dirp = reinterpret_cast( - reinterpret_cast(dirp) + written); + current_dirp = reinterpret_cast(reinterpret_cast(dirp) + written); current_dirp->d_ino = std::hash()( open_dir->path() + "/" + de.name()); diff --git a/src/client/hooks.cpp b/src/client/hooks.cpp index dc5db1377..fa437d1ed 100644 --- a/src/client/hooks.cpp +++ b/src/client/hooks.cpp @@ -25,7 +25,6 @@ extern "C" { #include -#include #include #include #include @@ -413,7 +412,7 @@ int hook_dup3(unsigned int oldfd, unsigned int newfd, int flags) { return syscall_no_intercept(SYS_dup3, oldfd, newfd, flags); } -int hook_getdents(unsigned int fd, struct dirent* dirp, unsigned int count) { +int hook_getdents(unsigned int fd, struct linux_dirent* dirp, unsigned int count) { LOG(DEBUG, "{}() called with fd: {}, dirp: {}, count: {}", __func__, fd, fmt::ptr(dirp), count); @@ -425,7 +424,7 @@ int hook_getdents(unsigned int fd, struct dirent* dirp, unsigned int count) { } -int hook_getdents64(unsigned int fd, struct dirent64* dirp, unsigned int count) { +int hook_getdents64(unsigned int fd, struct linux_dirent64* dirp, unsigned int count) { LOG(DEBUG, "{}() called with fd: {}, dirp: {}, count: {}", __func__, fd, fmt::ptr(dirp), count); diff --git a/src/client/intercept.cpp b/src/client/intercept.cpp index 56471c712..9905a0b5d 100644 --- a/src/client/intercept.cpp +++ b/src/client/intercept.cpp @@ -596,13 +596,13 @@ int hook(long syscall_number, case SYS_getdents: *result = gkfs::hook::hook_getdents(static_cast(arg0), - reinterpret_cast(arg1), + reinterpret_cast(arg1), static_cast(arg2)); break; case SYS_getdents64: *result = gkfs::hook::hook_getdents64(static_cast(arg0), - reinterpret_cast(arg1), + reinterpret_cast(arg1), static_cast(arg2)); break; -- GitLab