summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRavishankar N <ranaraya@redhat.com>2014-01-29 12:09:42 +0000
committerVijay Bellur <vbellur@redhat.com>2014-02-17 01:51:03 -0800
commit5b2d308f4b3969e37ceca7648165add1badb0de3 (patch)
treee127a2740ce615e41087267a0ac545e94c8a9f8c
parentb319f01ecd8568659f365f4fa6bbed0acf06db76 (diff)
storage/posix: perform chmod after chown.
Problem: When a replica brick is added to a volume, set-user-ID and set-group-ID permission bits of files are not set correctly in the new brick. The issue is in the posix_setattr() call where we do a chmod followed by a chown. But according to the man pages for chown: When the owner or group of an executable file are changed by an unprivileged user the S_ISUID and S_ISGID mode bits are cleared. POSIX does not specify whether this also should happen when root does the chown(). Fix: Swap the chmod and chown calls in posix_setattr() BUG: 1058797 Change-Id: Id2fbd8394cf6faf669f414775409f20f46009f2b Signed-off-by: Ravishankar N <ravishankar@redhat.com> Reviewed-on: http://review.gluster.org/6988 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--tests/bugs/bug-1058797.t45
-rw-r--r--xlators/storage/posix/src/posix.c12
2 files changed, 51 insertions, 6 deletions
diff --git a/tests/bugs/bug-1058797.t b/tests/bugs/bug-1058797.t
new file mode 100644
index 0000000..2b80794
--- /dev/null
+++ b/tests/bugs/bug-1058797.t
@@ -0,0 +1,45 @@
+#!/bin/bash
+#Test that the setuid bit is healed correctly.
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+#Basic checks
+TEST glusterd
+
+#Create a 1x2 replica volume
+TEST $CLI volume create $V0 replica 2 $H0:$B0/brick{0,1};
+TEST $CLI volume start $V0
+TEST $CLI volume set $V0 cluster.self-heal-daemon off
+
+# FUSE mount;create a file
+TEST glusterfs -s $H0 --volfile-id $V0 $M0
+TEST touch $M0/file
+
+#Kill brick1 and set S_ISUID and S_ISGID bits from mount point
+kill_brick $V0 $H0 $B0/brick1
+TEST chmod +x,+s $M0/file
+
+#Get file permissions from backend brick0 and verify that S_ISUID is indeed set
+file_permissions1=`ls -l $B0/brick0/file | awk '{print $1}'| cut -d. -f1 | cut -d- -f2,3,4,5,6`
+setuid_bit1=`echo $file_permissions1 | cut -b3`
+EXPECT "s" echo $setuid_bit1
+
+#Restart volume and do lookup from mount to trigger heal
+TEST $CLI volume start $V0 force
+EXPECT_WITHIN 20 "1" afr_child_up_status $V0 1
+TEST ls -l $M0/file
+
+#Get file permissions from healed brick1 and verify that S_ISUID is indeed set
+file_permissions2=`ls -l $B0/brick1/file | awk '{print $1}' | cut -d. -f1 | cut -d- -f2,3,4,5,6`
+setuid_bit2=`echo $file_permissions2 | cut -b3`
+EXPECT "s" echo $setuid_bit2
+
+#Also compare the entire permission string,just to be sure
+EXPECT $file_permissions1 echo $file_permissions2
+TEST umount $M0
+TEST $CLI volume stop $V0
+TEST $CLI volume delete $V0;
+
+cleanup;
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c
index 2abe928..d134ece 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -362,23 +362,23 @@ posix_setattr (call_frame_t *frame, xlator_t *this,
goto out;
}
- if (valid & GF_SET_ATTR_MODE) {
- op_ret = posix_do_chmod (this, real_path, stbuf);
+ if (valid & (GF_SET_ATTR_UID | GF_SET_ATTR_GID)){
+ op_ret = posix_do_chown (this, real_path, stbuf, valid);
if (op_ret == -1) {
op_errno = errno;
gf_log (this->name, GF_LOG_ERROR,
- "setattr (chmod) on %s failed: %s", real_path,
+ "setattr (chown) on %s failed: %s", real_path,
strerror (op_errno));
goto out;
}
}
- if (valid & (GF_SET_ATTR_UID | GF_SET_ATTR_GID)){
- op_ret = posix_do_chown (this, real_path, stbuf, valid);
+ if (valid & GF_SET_ATTR_MODE) {
+ op_ret = posix_do_chmod (this, real_path, stbuf);
if (op_ret == -1) {
op_errno = errno;
gf_log (this->name, GF_LOG_ERROR,
- "setattr (chown) on %s failed: %s", real_path,
+ "setattr (chmod) on %s failed: %s", real_path,
strerror (op_errno));
goto out;
}