chmod05: fix to modify the group ownership before do dir chmod

PATCH In review Wei Yongjun 2 participants 4 messages Jun 8, 2009 – Jun 9, 2009

lore

Share & tools

lore

Wei Yongjun

author

2009-06-08 06:17 UTC #0 lore raw

PATCH 1/2 chmod05: fix to modify the group ownership before do dir chmod

Refer to the manpage:

  # man 2 chmod
  If the calling process is not privileged (Linux: does not have the
  CAP_FSETID capability), and the group of the file does not match
  the effective group ID of the process or one of its supplementary
  group IDs, the S_ISGID bit will be turned off, but this will not
  cause an error to be returned.

So, if we want S_ISGID bit be turned off after chmod(), we can not have the CAP_FSETID capability and not match the effective group ID. The 'bin' group always has the CAP_FSETID capability, so we can not change the own of the TESTDIR to 'bin' group, instead, 'nobody' can be used.

This patch fixed the problem by change gid of chown to 'nobody' group and change the gid of setegid() to 'bin' group.

Signed-off-by Wei Yongjun <[email protected]>

---
 testcases/kernel/syscalls/chmod/chmod05.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

testcases/kernel/syscalls/chmod/chmod05.c +4 -4

index 4504aaa..c6f1225 100644
@@ -177,7 +177,7 @@ int main(int ac, char **av)
 			if ((PERMS & ~S_ISGID) != dir_mode) {
 				tst_resm(TFAIL, "%s: Incorrect modes 0%03o, "
 					 "Expected 0%03o", TESTDIR, dir_mode,
-					 PERMS);
+					 PERMS & ~S_ISGID);
 			} else {
 				tst_resm(TPASS,
 					 "Functionality of chmod(%s, %#o) successful",

@@ -241,12 +241,12 @@ void setup()
 				strerror(errno));
 	}
 
-	if (chown(TESTDIR, nobody_u->pw_uid, bin_group->gr_gid) == -1)
+	if (chown(TESTDIR, nobody_u->pw_uid, nobody_u->pw_gid) == -1)
 		tst_brkm(TBROK, cleanup, "Couldn't change owner of testdir: %s",
 				strerror(errno));
 
-	/* change to nobody:nobody */
-	if (setegid(nobody_u->pw_gid) == -1 ||
+	/* change to nobody:bin */
+	if (setegid(bin_group->gr_gid) == -1 ||
 		 seteuid(nobody_u->pw_uid) == -1)
 		tst_brkm(TBROK, cleanup, "Couldn't switch to nobody:nobody: %s",
 				strerror(errno));

Signature · 16 lines

1.6.0.2.530.g67faa





------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Wei Yongjun

author

2009-06-08 06:20 UTC #1 lore raw

PATCH 2/2 chmod05: fix the effective user when do cleanup

The TESTDIR is created by root user, but when we do cleanup, the effective user had been changed to nobody, so the cleanup will be failed when the TESTDIR is removed.

  chmod05     0  WARN  :  tst_rmdir(): rmobj(/tmp/chmOpEdLA) failed: \
  remove(/tmp/chmOpEdLA) failed; errno=1: Operation not permitted

This patch fixed the problem by reset the effective user to root.

Signed-off-by Wei Yongjun <[email protected]>

---
 testcases/kernel/syscalls/chmod/chmod05.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

testcases/kernel/syscalls/chmod/chmod05.c +3 -0

index c6f1225..252f14e 100644
@@ -266,6 +266,9 @@ void cleanup()
 	 */
 	TEST_CLEANUP;
 
+	setegid(0);
+	seteuid(0);
+
 	/* Remove tmp dir and all files in it */
 	tst_rmdir();

Signature · 16 lines

1.6.0.2.530.g67faa





------------------------------------------------------------------------------
OpenSolaris 2009.06 is a cutting edge operating system for enterprises 
looking to deploy the next generation of Solaris that includes the latest 
innovations from Sun and the OpenSource community. Download a copy and 
enjoy capabilities such as Networking, Storage and Virtualization. 
Go to: http://p.sf.net/sfu/opensolaris-get
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Subrata Modak

2009-06-09 18:25 UTC #3 lore raw

In reply to Wei Yongjun

Quoted 14 lines · Wei Yongjun

Refer to the manpage:
  # man 2 chmod
  If the calling process is not privileged (Linux: does not have the
  CAP_FSETID capability), and the group of the file does not match
  the effective group ID of the process or one of its supplementary
  group IDs, the S_ISGID bit will be turned off, but this will not
  cause an error to be returned.
So, if we want S_ISGID bit be turned off after chmod(), we can not have the CAP_FSETID capability and not match the effective group ID. The 'bin' group always has the CAP_FSETID capability, so we can not change the own of the TESTDIR to 'bin' group, instead, 'nobody' can be used.

Quoted 4 lines

This patch fixed the problem by change gid of chown to 'nobody' group and change the gid of setegid() to 'bin' group.
Signed-off-by Wei Yongjun <[email protected]>

Thanks.

Regards--
Subrata

Quoted 23 lines

---
 testcases/kernel/syscalls/chmod/chmod05.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

testcases/kernel/syscalls/chmod/chmod05.c +2 -2

index 4504aaa..c6f1225 100644
@@ -177,7 +177,7 @@ int main(int ac, char **av)
 			if ((PERMS & ~S_ISGID) != dir_mode) {
 				tst_resm(TFAIL, "%s: Incorrect modes 0%03o, "
 					 "Expected 0%03o", TESTDIR, dir_mode,
-					 PERMS);
+					 PERMS & ~S_ISGID);
 			} else {
 				tst_resm(TPASS,
 					 "Functionality of chmod(%s, %#o) successful",

@@ -241,12 +241,12 @@ void setup()
 				strerror(errno));
 	}
 
-	if (chown(TESTDIR, nobody_u->pw_uid, bin_group->gr_gid) == -1)
+	if (chown(TESTDIR, nobody_u->pw_uid, nobody_u->pw_gid) == -1)

testcases/kernel/syscalls/chmod/chmod05.c void setup() · line 252

Quoted 10 lines

testcases/kernel/syscalls/chmod/chmod05.c +2 -2

 		tst_brkm(TBROK, cleanup, "Couldn't change owner of testdir: %s",
 				strerror(errno));
 
-	/* change to nobody:nobody */
-	if (setegid(nobody_u->pw_gid) == -1 ||
+	/* change to nobody:bin */
+	if (setegid(bin_group->gr_gid) == -1 ||
 		 seteuid(nobody_u->pw_uid) == -1)
 		tst_brkm(TBROK, cleanup, "Couldn't switch to nobody:nobody: %s",
 				strerror(errno));

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Subrata Modak

2009-06-09 18:25 UTC #2 lore raw

Re: [LTP] [PATCH 2/2] chmod05: fix the effective user when do cleanup

In reply to Wei Yongjun

Quoted 10 lines · Wei Yongjun

The TESTDIR is created by root user, but when we do cleanup, the effective user had been changed to nobody, so the cleanup will be failed when the TESTDIR is removed.
  chmod05     0  WARN  :  tst_rmdir(): rmobj(/tmp/chmOpEdLA) failed: \
  remove(/tmp/chmOpEdLA) failed; errno=1: Operation not permitted
This patch fixed the problem by reset the effective user to root.
Signed-off-by Wei Yongjun <[email protected]>

Thanks for fixing this as well.

Regards--
Subrata

Quoted 9 lines

---
 testcases/kernel/syscalls/chmod/chmod05.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

testcases/kernel/syscalls/chmod/chmod05.c +0 -0

index c6f1225..252f14e 100644

testcases/kernel/syscalls/chmod/chmod05.c void cleanup() · line 274

Quoted 9 lines

testcases/kernel/syscalls/chmod/chmod05.c +3 -0

@@ -266,6 +266,9 @@ void cleanup()
 	 */
 	TEST_CLEANUP;
 
+	setegid(0);
+	seteuid(0);
+
 	/* Remove tmp dir and all files in it */
 	tst_rmdir();