From 3af42583dd804371952d61e9d7ff4c640e67ba0d Mon Sep 17 00:00:00 2001 From: Vijay Bellur Date: Sun, 12 Jan 2014 22:39:14 +0530 Subject: storage/posix: UNWIND right op_error and op_errno in *setxattr() 1. errno was being set after gf_log() in posix_{f}handle_pair, this would cause errno to be overwritten. 2. dht would expect -1 for indication of failure in setxattr callback (dht_err_cbk()). posix_{f}setxattr has been changed to set op_ret as -1 instead of -op_errno. 3. dict_foreach() has been changed to return an error if the invoked fn() returns < 0. Bug report and test case credits to Zorro Lang Change-Id: I96c15f12a5d7717b7584ba392f390a0b4f704a98 BUG: 1051896 Signed-off-by: Vijay Bellur Reviewed-on: http://review.gluster.org/6684 Tested-by: Gluster Build System Reviewed-by: Niels de Vos Reviewed-by: Anand Avati --- libglusterfs/src/dict.c | 4 +- tests/bugs/bug-1051896.c | 94 +++++++++++++++++++++++++++++++ tests/bugs/bug-1051896.t | 24 ++++++++ xlators/storage/posix/src/posix-helpers.c | 4 +- xlators/storage/posix/src/posix.c | 8 ++- 5 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 tests/bugs/bug-1051896.c create mode 100644 tests/bugs/bug-1051896.t diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c index f2df5a6d4..e9fc1222d 100644 --- a/libglusterfs/src/dict.c +++ b/libglusterfs/src/dict.c @@ -1121,8 +1121,8 @@ dict_foreach (dict_t *dict, while (pairs) { next = pairs->next; ret = fn (dict, pairs->key, pairs->value, data); - if (ret == -1) - return -1; + if (ret < 0) + return ret; pairs = next; } diff --git a/tests/bugs/bug-1051896.c b/tests/bugs/bug-1051896.c new file mode 100644 index 000000000..0ffd81986 --- /dev/null +++ b/tests/bugs/bug-1051896.c @@ -0,0 +1,94 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +int do_setfacl(const char *path, const char *options, const char *textacl) +{ + int r; + int type; + acl_t acl; + int dob; + int dok; + int dom; + struct stat st; + char textmode[30]; + + r = 0; + dob = strchr(options,'b') != (char*)NULL; + dok = strchr(options,'k') != (char*)NULL; + dom = strchr(options,'m') != (char*)NULL; + if ((dom && !textacl) + || (!dom && (textacl || (!dok && !dob) || + strchr(options,'d')))) { + errno = EBADRQC; /* "bad request" */ + r = -1; + } else { + if (dob || dok) { + r = acl_delete_def_file(path); + } + if (dob && !r) { + if (!stat(path,&st)) { + sprintf(textmode, + "u::%c%c%c,g::%c%c%c,o::%c%c%c", + (st.st_mode & 0400 ? 'r' : '-'), + (st.st_mode & 0200 ? 'w' : '-'), + (st.st_mode & 0100 ? 'x' : '-'), + (st.st_mode & 0040 ? 'r' : '-'), + (st.st_mode & 0020 ? 'w' : '-'), + (st.st_mode & 0010 ? 'x' : '-'), + (st.st_mode & 004 ? 'r' : '-'), + (st.st_mode & 002 ? 'w' : '-'), + (st.st_mode & 001 ? 'x' : '-')); + acl = acl_from_text(textmode); + if (acl) { + r = acl_set_file(path, + ACL_TYPE_ACCESS,acl); + acl_free(acl); + } else + r = -1; + } else + r = -1; + } + if (!r && dom) { + if (strchr(options,'d')) + type = ACL_TYPE_DEFAULT; + else + type = ACL_TYPE_ACCESS; + acl = acl_from_text(textacl); + if (acl) { + r = acl_set_file(path,type,acl); + acl_free(acl); + } else + r = -1; + } + } + if (r) + r = -errno; + return (r); +} + + +int main(int argc, char *argv[]){ + int rc = 0; + if (argc != 4) { + fprintf(stderr, + "usage: ./setfacl_test \n"); + return 0; + } + if ((rc = do_setfacl(argv[1], argv[2], argv[3])) != 0){ + fprintf(stderr, "do_setfacl failed: %s\n", strerror(errno)); + return rc; + } + return 0; +} diff --git a/tests/bugs/bug-1051896.t b/tests/bugs/bug-1051896.t new file mode 100644 index 000000000..75859cbef --- /dev/null +++ b/tests/bugs/bug-1051896.t @@ -0,0 +1,24 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc +. $(dirname $0)/../volume.rc + +cleanup; + +## Start and create a volume +TEST glusterd; +TEST pidof glusterd; +TEST $CLI volume info; + +TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{1,2,3,4}; +TEST $CLI volume start $V0; + +TEST glusterfs --entry-timeout=0 --attribute-timeout=0 --acl -s $H0 --volfile-id $V0 $M0; + +TEST touch $M0/file1; + +gcc -lacl $(dirname $0)/bug-1051896.c -o $(dirname $0)/bug-1051896 +TEST ! $(dirname $0)/bug-1051896 $M0/file1 m 'u::r,u::w,g::r--,o::r--' +rm -f $(dirname $0)/bug-1051896 + +cleanup diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index d2c991900..3a66ecfc2 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -792,6 +792,7 @@ posix_handle_pair (xlator_t *this, const char *real_path, value->len, flags); if (sys_ret < 0) { + ret = -errno; if (errno == ENOTSUP) { GF_LOG_OCCASIONALLY(gf_xattr_enotsup_log, this->name,GF_LOG_WARNING, @@ -823,7 +824,6 @@ posix_handle_pair (xlator_t *this, const char *real_path, #endif /* DARWIN */ } - ret = -errno; goto out; } } @@ -847,6 +847,7 @@ posix_fhandle_pair (xlator_t *this, int fd, value->len, flags); if (sys_ret < 0) { + ret = -errno; if (errno == ENOTSUP) { GF_LOG_OCCASIONALLY(gf_xattr_enotsup_log, this->name,GF_LOG_WARNING, @@ -873,7 +874,6 @@ posix_fhandle_pair (xlator_t *this, int fd, #endif /* DARWIN */ } - ret = -errno; goto out; } diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index 7695289fa..dc4af1b92 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -2950,8 +2950,10 @@ posix_setxattr (call_frame_t *frame, xlator_t *this, filler.flags = flags; op_ret = dict_foreach (dict, _handle_setxattr_keyvalue_pair, &filler); - if (op_ret < 0) + if (op_ret < 0) { op_errno = -op_ret; + op_ret = -1; + } out: SET_TO_OLD_FS_ID (); @@ -3916,8 +3918,10 @@ posix_fsetxattr (call_frame_t *frame, xlator_t *this, filler.flags = flags; op_ret = dict_foreach (dict, _handle_fsetxattr_keyvalue_pair, &filler); - if (op_ret < 0) + if (op_ret < 0) { op_errno = -op_ret; + op_ret = -1; + } out: SET_TO_OLD_FS_ID (); -- cgit