From 16230023e5afcb0b42b8d01207e3449d22772c31 Mon Sep 17 00:00:00 2001

From: Brandon Philips <brandon@ifup.org>

Date: Thu, 17 Dec 2009 14:28:04 -0800

Subject: [PATCH] setfacl: changing owner and when S_ISUID should be set --restore fix

Fix a problem in setfacl --restore when the owner or group is changed

and the S_ISUID and S_ISGID are to be set.

The root of the problem is that chown() can clear the S_ISUID and

S_ISGID bits as described in chown(2):

 When  the  owner  or  group of an executable file are changed by a

 non- superuser, the S_ISUID and S_ISGID mode bits are cleared.   POSIX

 does not specify whether this also should happen when root does the

 chown(); the Linux behavior depends on the kernel version.  In case  of

 a  non- group-executable  file (i.e., one for which the S_IXGRP bit is

 not set) the S_ISGID bit indicates mandatory locking, and is not

 cleared  by  a chown().

To fix the issue re-stat() the file after chown() so that the logic

surrounding the chmod() has the updated mode of the file.

Signed-off-by: Brandon Philips <bphilips@suse.de>

---

 setfacl/setfacl.c      |    8 +++++++-

 test/root/restore.test |   23 +++++++++++++++++++++++

 2 files changed, 30 insertions(+), 1 deletions(-)

 create mode 100644 test/root/restore.test

diff --git a/setfacl/setfacl.c b/setfacl/setfacl.c

index 091b9cc..56b0aa4 100644

--- a/setfacl/setfacl.c

+++ b/setfacl/setfacl.c

@@ -128,6 +128,7 @@ restore(

 	struct do_set_args args;

 	int line = 0, backup_line;

 	int error, status = 0;

+	int chmod_required = 0;

 	memset(&st, 0, sizeof(st));

@@ -206,10 +207,15 @@ restore(

 					strerror(errno));

 				status = 1;

 			}

+

+			/* chown() clears setuid/setgid so force a chmod if

+			 * S_ISUID/S_ISGID was expected */

+			if ((st.st_mode & flags) & (S_ISUID | S_ISGID))

+				chmod_required = 1;

 		}

 		mask = S_ISUID | S_ISGID | S_ISVTX;

-		if ((st.st_mode & mask) != (flags & mask)) {

+		if (chmod_required || ((st.st_mode & mask) != (flags & mask))) {

 			if (!args.mode)

 				args.mode = st.st_mode;

 			args.mode &= (S_IRWXU | S_IRWXG | S_IRWXO);

diff --git a/test/root/restore.test b/test/root/restore.test

new file mode 100644

index 0000000..6003cd4

--- /dev/null

+++ b/test/root/restore.test

@@ -0,0 +1,23 @@

+Ensure setuid bit is restored when the owner changes

+ https://bugzilla.redhat.com/show_bug.cgi?id=467936#c7

+

+	$ touch passwd

+	$ chmod 755 passwd

+	$ chmod u+s passwd

+	$ getfacl passwd > passwd.acl

+	$ cat passwd.acl

+	> # file: passwd

+	> # owner: root

+	> # group: root

+	> # flags: s--

+	> user::rwx

+	> group::r-x

+	> other::r-x

+	>

+	$ chown bin passwd

+	$ chmod u+s passwd

+	$ setfacl --restore passwd.acl

+	$ ls -dl passwd | awk '{print $1 " " $3 " " $4}'

+	> -rwsr-xr-x root root

+

+	$ rm passwd passwd.acl

-- 

1.6.2.5