From 16230023e5afcb0b42b8d01207e3449d22772c31 Mon Sep 17 00:00:00 2001 From: Brandon Philips 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 --- 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