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