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

Marc Vefrequested to merge
78-errno-115-returned-on-successful-completion-for-several-system-calls into master

Closes #78 (closed)

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.

Merge request reports

src/client/gkfs_functions.cpp

+4 −2
Changes for src/client/gkfs_functions.cpp: 4 added lines, 2 removed lines.
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
Changes for src/client/hooks.cpp: 1 added line, 0 removed lines.
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
Changes for src/client/preload.cpp: 3 added lines, 0 removed lines.
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
Changes for tests/integration/data/test_data_integrity.py: 0 added lines, 1 removed line.
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
Changes for tests/integration/data/test_truncate.py: 0 added lines, 12 removed lines.
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!