diff options
author | Brandon Philips <brandon@ifup.org> | 2009-12-17 14:28:04 -0800 |
---|---|---|
committer | Brandon Philips <brandon@ifup.org> | 2009-12-17 14:28:04 -0800 |
commit | 16230023e5afcb0b42b8d01207e3449d22772c31 (patch) | |
tree | 02ef1059f60fb4aa90a5dd3fa60cbeb3bdddb368 | |
parent | 05c5bdcb4dc865c5822d7da7d0043daf2076ed46 (diff) | |
download | acl-16230023e5afcb0b42b8d01207e3449d22772c31.tar.gz |
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>
-rw-r--r-- | setfacl/setfacl.c | 8 | ||||
-rw-r--r-- | test/root/restore.test | 23 |
2 files changed, 30 insertions, 1 deletions
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 |