summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKrutika Dhananjay <kdhananj@redhat.com>2015-08-19 16:54:42 +0530
committerPranith Kumar Karampuri <pkarampu@redhat.com>2015-08-31 02:31:10 -0700
commitf0afc2e7c7bee3043ac1e7d098bc44db60186856 (patch)
treeb3b63b9de35cd9f12f8cfead7c53a4a96ba21d75
parent889f9511a46c5e02007f70f567d2e0eaa2c9f41a (diff)
features/shard: Fix permission issues
Backport of: http://review.gluster.org/11992 This patch does the following: * reverts commit b467af0e99b39ef708420d3f7f6696b0ca618512 * changes ownership on shards under /.shard to be root:root * makes readv, writev, [f]truncate, rename, and unlink fops to perform operations on files under /.shard with frame->root->{uid,gid} as 0. This would ensure that a [f]setattr on a sharded file does not need to be called on all the shards associated with it. Change-Id: I50d8533bd2b769a4dfe8cd1b49bdcfc117a7e660 BUG: 1253151 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com> Reviewed-on: http://review.gluster.org/12052 Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com> Tested-by: NetBSD Build System <jenkins@build.gluster.org>
-rw-r--r--tests/bugs/shard/bug-1251824.t68
-rw-r--r--tests/common-utils.rc7
-rw-r--r--xlators/features/shard/src/shard.c47
-rw-r--r--xlators/features/shard/src/shard.h18
4 files changed, 121 insertions, 19 deletions
diff --git a/tests/bugs/shard/bug-1251824.t b/tests/bugs/shard/bug-1251824.t
index 71bfdc7fdae..9e6ceaa8161 100644
--- a/tests/bugs/shard/bug-1251824.t
+++ b/tests/bugs/shard/bug-1251824.t
@@ -1,6 +1,7 @@
#!/bin/bash
. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../common-utils.rc
cleanup;
@@ -8,52 +9,97 @@ TEST glusterd
TEST pidof glusterd
TEST $CLI volume create $V0 $H0:$B0/${V0}{0,1}
TEST $CLI volume set $V0 features.shard on
+TEST $CLI volume set $V0 performance.strict-write-ordering on
TEST $CLI volume start $V0
-TEST $GFS --volfile-id=$V0 --volfile-server=$H0 $M0
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0
TEST useradd -M test_user 2>/dev/null
# Create 3 files as root.
TEST touch $M0/foo
TEST touch $M0/bar
TEST touch $M0/baz
+TEST touch $M0/qux
+TEST mkdir $M0/dir
# Change ownership to non-root on foo and bar.
TEST chown test_user:test_user $M0/foo
TEST chown test_user:test_user $M0/bar
# Write 6M of data on foo as non-root, 2M overflowing into block-1.
-su -m test_user -c "dd if=/dev/zero of=$M0/foo bs=1M count=6"
+TEST run_cmd_as_user test_user "dd if=/dev/zero of=$M0/foo bs=1M count=6"
-# Ensure owner and group are same on the shard as the main file.
+# Ensure owner and group are root on the block-1 shard.
gfid_foo=`getfattr -n glusterfs.gfid.string $M0/foo 2>/dev/null \
| grep glusterfs.gfid.string | cut -d '"' -f 2`
-EXPECT "test_user" echo `find $B0 -name $gfid_foo.1 | xargs stat -c %U`
-EXPECT "test_user" echo `find $B0 -name $gfid_foo.1 | xargs stat -c %G`
+EXPECT "root" echo `find $B0 -name $gfid_foo.1 | xargs stat -c %U`
+EXPECT "root" echo `find $B0 -name $gfid_foo.1 | xargs stat -c %G`
+
+#Ensure /.shard is owned by root.
+EXPECT "root" echo `find $B0/${V0}0 -name .shard | xargs stat -c %U`
+EXPECT "root" echo `find $B0/${V0}0 -name .shard | xargs stat -c %G`
+EXPECT "root" echo `find $B0/${V0}1 -name .shard | xargs stat -c %U`
+EXPECT "root" echo `find $B0/${V0}1 -name .shard | xargs stat -c %G`
# Write 6M of data on bar as root.
TEST dd if=/dev/zero of=$M0/bar bs=1M count=6
-# Ensure owner and group are same on the shard as the main file.
+# Ensure owner and group are root on the block-1 shard.
gfid_bar=`getfattr -n glusterfs.gfid.string $M0/bar 2>/dev/null \
| grep glusterfs.gfid.string | cut -d '"' -f 2`
-EXPECT "test_user" echo `find $B0 -name $gfid_bar.1 | xargs stat -c %U`
-EXPECT "test_user" echo `find $B0 -name $gfid_bar.1 | xargs stat -c %G`
+EXPECT "root" echo `find $B0 -name $gfid_bar.1 | xargs stat -c %U`
+EXPECT "root" echo `find $B0 -name $gfid_bar.1 | xargs stat -c %G`
# Write 6M of data on baz as root.
TEST dd if=/dev/zero of=$M0/baz bs=1M count=6
-# Ensure owner andgroup are same on the shard as the main file.
gfid_baz=`getfattr -n glusterfs.gfid.string $M0/baz 2>/dev/null \
| grep glusterfs.gfid.string | cut -d '"' -f 2`
+# Ensure owner and group are root on the block-1 shard.
EXPECT "root" echo `find $B0 -name $gfid_baz.1 | xargs stat -c %U`
EXPECT "root" echo `find $B0 -name $gfid_baz.1 | xargs stat -c %G`
-userdel test_user
-TEST umount $M0
+# Test to ensure unlink from an unauthorized user does not lead to only
+# the shards under /.shard getting unlinked while that on the base file fails
+# with EPERM/ACCES.
+
+TEST ! run_cmd_as_user test_user "unlink $M0/baz"
+TEST find $B0/*/.shard/$gfid_baz.1
+
+# Test to ensure rename of a file where the dest file exists and is sharded,
+# from an unauthorized user does not lead to only the shards under /.shard
+# getting unlinked while that on the base file fails with EPERM/ACCES.
+
+TEST ! run_cmd_as_user test_user "mv -f $M0/qux $M0/baz"
+TEST find $B0/*/.shard/$gfid_baz.1
+TEST stat $M0/qux
+
+# Shard translator executes steps in the following order while doing a truncate
+# to a lower size:
+# 1) unlinking shards under /.shard first with frame->root->{uid,gid} being 0,
+# 2) truncate the original file by the right amount.
+# The following two tests are towards ensuring that truncate attempt from an
+# unauthorised user doesn't result in only the shards under /.shard getting
+# removed (since they're being performed as root) while step 2) above fails,
+# leaving the file in an inconsistent state.
+
+TEST ! run_cmd_as_user test_user "truncate -s 1M $M0/baz"
+TEST find $B0/*/.shard/$gfid_baz.1
+
+# Perform a cp as non-root user. This should trigger readv() which will trigger
+# reads on first shard of "foo" under /.shard, and this must not fail if shard
+# translator correctly sets frame->root->uid,gid to 0 before reading off the
+# first shard, since it's owned by root.
+TEST chown test_user:test_user $M0/dir
+TEST run_cmd_as_user test_user "cp $M0/foo $M0/dir/quux"
+
+md5sum_foo=$(md5sum $M0/foo | awk '{print $1}')
+EXPECT "$md5sum_foo" echo `md5sum $M0/dir/quux | awk '{print $1}'`
+
+userdel test_user
TEST $CLI volume stop $V0
TEST $CLI volume delete $V0
diff --git a/tests/common-utils.rc b/tests/common-utils.rc
new file mode 100644
index 00000000000..2be4076e8b6
--- /dev/null
+++ b/tests/common-utils.rc
@@ -0,0 +1,7 @@
+
+function run_cmd_as_user {
+ local user=$1
+ shift
+ su -m $user -c "$*" || return 1
+ return 0
+}
diff --git a/xlators/features/shard/src/shard.c b/xlators/features/shard/src/shard.c
index 2c893cc1101..6fb57168371 100644
--- a/xlators/features/shard/src/shard.c
+++ b/xlators/features/shard/src/shard.c
@@ -913,6 +913,8 @@ shard_truncate_last_shard_cbk (call_frame_t *frame, void *cookie,
local = frame->local;
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
+
if (op_ret < 0) {
local->op_ret = op_ret;
local->op_errno = op_errno;
@@ -948,6 +950,19 @@ shard_truncate_last_shard (call_frame_t *frame, xlator_t *this, inode_t *inode)
local = frame->local;
+ /* A NULL inode could be due to the fact that the last shard which
+ * needs to be truncated does not exist due to it lying in a hole
+ * region. So the only thing left to do in that case would be an
+ * update to file size xattr.
+ */
+ if (!inode) {
+ shard_update_file_size (frame, this, NULL, &local->loc,
+ shard_post_update_size_truncate_handler);
+ return 0;
+ }
+
+ SHARD_SET_ROOT_FS_ID (frame, local);
+
loc.inode = inode_ref (inode);
gf_uuid_copy (loc.gfid, inode->gfid);
@@ -1012,6 +1027,8 @@ shard_truncate_htol (call_frame_t *frame, xlator_t *this, inode_t *inode)
local->call_count = call_count;
i = 1;
+
+ SHARD_SET_ROOT_FS_ID (frame, local);
while (cur_block <= last_block) {
if (!local->inode_list[i]) {
cur_block++;
@@ -1145,7 +1162,9 @@ shard_common_lookup_shards_cbk (call_frame_t *frame, void *cookie,
if (op_ret < 0) {
/* Ignore absence of shards in the backend in truncate fop. */
if (((local->fop == GF_FOP_TRUNCATE) ||
- (local->fop == GF_FOP_FTRUNCATE)) && (op_errno == ENOENT))
+ (local->fop == GF_FOP_FTRUNCATE) ||
+ (local->fop == GF_FOP_RENAME) ||
+ (local->fop == GF_FOP_UNLINK)) && (op_errno == ENOENT))
goto done;
local->op_ret = op_ret;
local->op_errno = op_errno;
@@ -1711,6 +1730,8 @@ shard_unlink_shards_do_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
done:
call_count = shard_call_count_return (frame);
if (call_count == 0) {
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
+
if (local->fop == GF_FOP_UNLINK)
shard_unlink_base_file (frame, this);
else if (local->fop == GF_FOP_RENAME)
@@ -1768,6 +1789,8 @@ shard_unlink_shards_do (call_frame_t *frame, xlator_t *this, inode_t *inode)
return 0;
}
+ SHARD_SET_ROOT_FS_ID (frame, local);
+
while (cur_block <= last_block) {
/* The base file is unlinked in the end to mark the
* successful completion of the fop.
@@ -2377,6 +2400,7 @@ out:
fd_unref (anon_fd);
call_count = shard_call_count_return (frame);
if (call_count == 0) {
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
if (local->op_ret < 0) {
SHARD_STACK_UNWIND (readv, frame, local->op_ret,
local->op_errno, NULL, 0, NULL,
@@ -2423,6 +2447,8 @@ shard_readv_do (call_frame_t *frame, xlator_t *this)
remaining_size = local->total_size;
local->call_count = call_count = local->num_blocks;
+ SHARD_SET_ROOT_FS_ID (frame, local);
+
while (cur_block <= last_block) {
if (wind_failed) {
shard_readv_do_cbk (frame, (void *) (long) 0, this, -1,
@@ -2552,8 +2578,7 @@ shard_common_mknod_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
done:
call_count = shard_call_count_return (frame);
if (call_count == 0) {
- frame->root->uid = local->uid;
- frame->root->gid = local->gid;
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
local->post_mknod_handler (frame, this);
}
@@ -2587,10 +2612,7 @@ shard_common_resume_mknod (call_frame_t *frame, xlator_t *this,
call_count = local->call_count = local->create_count;
local->post_mknod_handler = post_mknod_handler;
- local->uid = frame->root->uid;
- local->gid = frame->root->gid;
- frame->root->uid = local->prebuf.ia_uid;
- frame->root->gid = local->prebuf.ia_gid;
+ SHARD_SET_ROOT_FS_ID (frame, local);
ret = shard_inode_ctx_get_all (fd->inode, this, &ctx_tmp);
if (ret) {
@@ -2676,6 +2698,7 @@ err:
* This block is for handling failure in shard_inode_ctx_get_all().
* Failures in the while-loop are handled within the loop.
*/
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
post_mknod_handler (frame, this);
return 0;
}
@@ -2925,6 +2948,7 @@ shard_writev_do_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
call_count = shard_call_count_return (frame);
if (call_count == 0) {
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
if (local->op_ret < 0) {
SHARD_STACK_UNWIND (writev, frame, local->written_size,
local->op_errno, NULL, NULL, NULL);
@@ -2974,10 +2998,13 @@ shard_writev_do (call_frame_t *frame, xlator_t *this)
local->call_count = call_count = local->num_blocks;
last_block = local->last_block;
+ SHARD_SET_ROOT_FS_ID (frame, local);
+
if (dict_set_uint32 (local->xattr_req,
GLUSTERFS_WRITE_UPDATE_ATOMIC, 4)) {
local->op_ret = -1;
local->op_errno = ENOMEM;
+ local->call_count = 1;
shard_writev_do_cbk (frame, (void *)(long)0, this, -1, ENOMEM,
NULL, NULL, NULL);
return 0;
@@ -3152,6 +3179,8 @@ shard_writev_mkdir_dot_shard_cbk (call_frame_t *frame, void *cookie,
local = frame->local;
+ SHARD_UNSET_ROOT_FS_ID (frame, local);
+
if (op_ret == -1) {
if (op_errno != EEXIST) {
goto unwind;
@@ -3201,9 +3230,11 @@ shard_writev_mkdir_dot_shard (call_frame_t *frame, xlator_t *this)
goto err;
}
+ SHARD_SET_ROOT_FS_ID (frame, local);
+
STACK_WIND (frame, shard_writev_mkdir_dot_shard_cbk, FIRST_CHILD(this),
FIRST_CHILD(this)->fops->mkdir, &local->dot_shard_loc,
- 0777, 0, xattr_req);
+ 0755, 0, xattr_req);
dict_unref (xattr_req);
return 0;
diff --git a/xlators/features/shard/src/shard.h b/xlators/features/shard/src/shard.h
index f480c2912b6..e859b96c4db 100644
--- a/xlators/features/shard/src/shard.h
+++ b/xlators/features/shard/src/shard.h
@@ -116,6 +116,23 @@
} \
} while (0)
+#define SHARD_SET_ROOT_FS_ID(frame, local) do { \
+ if (!local->is_set_fsid) { \
+ local->uid = frame->root->uid; \
+ local->gid = frame->root->gid; \
+ frame->root->uid = 0; \
+ frame->root->gid = 0; \
+ local->is_set_fsid = _gf_true; \
+ } \
+} while (0)
+
+#define SHARD_UNSET_ROOT_FS_ID(frame, local) do { \
+ if (local->is_set_fsid) { \
+ frame->root->uid = local->uid; \
+ frame->root->gid = local->gid; \
+ local->is_set_fsid = _gf_false; \
+ } \
+} while (0)
typedef struct shard_priv {
uint64_t block_size;
@@ -184,6 +201,7 @@ typedef struct shard_local {
struct iobref *iobref;
struct iobuf *iobuf;
gf_dirent_t entries_head;
+ gf_boolean_t is_set_fsid;
gf_boolean_t list_inited;
gf_boolean_t is_write_extending;
shard_post_fop_handler_t handler;