Verified Commit 4f9cb830 authored by Alberto Miranda's avatar Alberto Miranda ♨️
Browse files

Merge branch... 

Merge branch '78-errno-115-returned-on-successful-completion-for-several-system-calls' into 'master'

This fix contains three parts:

1. errno 115 was leaked during initialization of the client in
   init_ld_environment_(). However, the value on its own is irrelevant to
   this issue as all internal errno variable changes should be kept
   internal as it is the case during each syscall intercept hook.
   Therefore, the original errno variable is restored after the
   initialization finishes.
2. lseek() tests revealed that whence SEEK_SET was not handled
   correctly. It should not allow negative offsets.

3. All FIXME TODOs in the testing code have been removed including
   testing on the errno in a successful case because it is undefined.

Resolve "Errno 115 returned on successful completion for several system calls"

Closes #78

See merge request !65
parents 4e873c61 9608ff14

src/client/gkfs_functions.cpp

+4 −2
Original line number Diff line number Diff line
@@ -418,6 +418,10 @@ off_t gkfs_lseek(unsigned int fd, off_t offset, unsigned int whence) { 
off_t gkfs_lseek(shared_ptr<gkfs::filemap::OpenFile> gkfs_fd, off_t offset, unsigned int whence) {

    switch (whence) {

        case SEEK_SET:

            if (offset < 0) {

                errno = EINVAL;

                return -1;

            }

            gkfs_fd->pos(offset);

            break;

        case SEEK_CUR:
@@ -1017,7 +1021,6 @@ int gkfs_getdents64(unsigned int fd, 
 * @return 0 on success or -1 on error

 */

int gkfs_mk_symlink(const std::string& path, const std::string& target_path) {

    gkfs::preload::init_ld_env_if_needed();

    /* The following check is not POSIX compliant.

     * In POSIX the target is not checked at all.

    *  Here if the target is a directory we raise a NOTSUP error.
@@ -1064,7 +1067,6 @@ int gkfs_mk_symlink(const std::string& path, const std::string& target_path) { 
 * @return 0 on success or -1 on error

 */

int gkfs_readlink(const std::string& path, char* buf, int bufsize) {

    gkfs::preload::init_ld_env_if_needed();

    auto md = gkfs::util::get_metadata(path, false);

    if (md == nullptr) {

        LOG(DEBUG, "Named link doesn't exist");

src/client/hooks.cpp

+1 −0
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ extern "C" { 


namespace {



// TODO replace all internal gkfs errno variable usage with LEAF

inline int with_errno(int ret) {

    return (ret < 0) ? -errno : ret;

}

src/client/preload.cpp

+3 −0
Original line number Diff line number Diff line
@@ -220,6 +220,8 @@ void init_ld_env_if_needed() { 
 * Called initially ONCE when preload library is used with the LD_PRELOAD environment variable

 */

void init_preload() {

    // The original errno value will be restored after initialization to not leak internal error codes

    auto oerrno = errno;



    CTX->enable_interception();

    gkfs::preload::start_self_interception();
@@ -252,6 +254,7 @@ void init_preload() { 
#endif



    gkfs::preload::start_interception();

    errno = oerrno;

}



/**

tests/integration/data/test_data_integrity.py

+0 −1
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ def test_data_integrity(gkfs_daemon, gkfs_client): 
            stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO)



    assert ret.retval == 0

    assert ret.errno == 115 #FIXME: Should be 0!



    # test stat on existing dir

    ret = gkfs_client.stat(topdir)

tests/integration/data/test_truncate.py

+0 −12
Original line number Diff line number Diff line
@@ -37,11 +37,9 @@ def test_truncate(gkfs_daemon, gkfs_client): 
    ret = gkfs_client.write_random(truncfile, buf_length)



    assert ret.retval == buf_length

    assert ret.errno == 115  # FIXME: Should be 0!



    ret = gkfs_client.stat(truncfile)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.statbuf.st_size == buf_length



    # truncate it
@@ -49,12 +47,10 @@ def test_truncate(gkfs_daemon, gkfs_client): 
    trunc_size = buf_length // 2

    ret = gkfs_client.truncate(truncfile, trunc_size)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.retval == 0

    # check file length

    ret = gkfs_client.stat(truncfile)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.statbuf.st_size == trunc_size



    # verify contents by writing a new file (random content is seeded) and checksum both
@@ -69,30 +65,25 @@ def test_truncate(gkfs_daemon, gkfs_client): 
    ret = gkfs_client.write_random(truncfile_verify, trunc_size)



    assert ret.retval == trunc_size

    assert ret.errno == 115  # FIXME: Should be 0!



    ret = gkfs_client.stat(truncfile_verify)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.statbuf.st_size == trunc_size



    ret = gkfs_client.file_compare(truncfile, truncfile_verify, trunc_size)



    assert ret.retval == 0

    assert ret.errno == 115  # FIXME: Should be 0!



    # trunc at byte 712345 (middle of chunk)

    # TODO feed chunksize into test to make sure it is always in the middle of the chunk

    trunc_size = 712345

    ret = gkfs_client.truncate(truncfile, trunc_size)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.retval == 0



    # check file length

    ret = gkfs_client.stat(truncfile)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.statbuf.st_size == trunc_size



    # verify contents by writing a new file (random content is seeded) and checksum both
@@ -107,14 +98,11 @@ def test_truncate(gkfs_daemon, gkfs_client): 
    ret = gkfs_client.write_random(truncfile_verify_2, trunc_size)



    assert ret.retval == trunc_size

    assert ret.errno == 115  # FIXME: Should be 0!



    ret = gkfs_client.stat(truncfile_verify_2)



    assert ret.errno == 115  # FIXME: Should be 0!

    assert ret.statbuf.st_size == trunc_size



    ret = gkfs_client.file_compare(truncfile, truncfile_verify_2, trunc_size)



    assert ret.retval == 0
 
    assert ret.errno == 115  # FIXME: Should be 0!