It can be seen here: https://semaphoreci.com/rpm-ecosystem/rpm/branches/pull-request-473/builds/1 If we call eu-elfcompress -q -p -t none "$f", it removes suid bit.
Simpler replicator: [root@tarox tmp]# cp /bin/echo ./echo-test [root@tarox tmp]# chmod +s ./echo-test [root@tarox tmp]# ls -l ./echo-test -rwsr-sr-x. 1 root root 33128 Jul 21 13:12 ./echo-test [root@tarox tmp]# eu-elfcompress -v -p -t none ./echo-test processing: ./echo-test [root@tarox tmp]# ls -l ./echo-test -rwxr-xr-x. 1 root root 33128 Jul 21 13:13 ./echo-test This is probably caused by the order eu-elfcompress sets the mode/owner of the file. fchown says: When the owner or group of an executable file are changed by an unpriv‐ ileged user 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(). and fchmod says: 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 even with eu-elfcompress -v you won't see any errors, but the permission bit might be dropped anyway. This patch solves the issue by swapping the fchmod and fchown calls: diff --git a/src/elfcompress.c b/src/elfcompress.c index bdb0e3b..1a0f984 100644 --- a/src/elfcompress.c +++ b/src/elfcompress.c @@ -1235,13 +1235,16 @@ process_file (const char *fname) elf_end (elfnew); elfnew = NULL; - /* Try to match mode and owner.group of the original file. */ - if (fchmod (fdnew, st.st_mode & ALLPERMS) != 0) - if (verbose >= 0) - error (0, errno, "Couldn't fchmod %s", fnew); + /* Try to match mode and owner.group of the original file. + Note to set suid bits we have to make sure the owner is setup + correctly first. Otherwise fchmod will drop them silently + or fchown may clear them. */ if (fchown (fdnew, st.st_uid, st.st_gid) != 0) if (verbose >= 0) error (0, errno, "Couldn't fchown %s", fnew); + if (fchmod (fdnew, st.st_mode & ALLPERMS) != 0) + if (verbose >= 0) + error (0, errno, "Couldn't fchmod %s", fnew); /* Finally replace the old file with the new file. */ if (foutput == NULL)
Why does it even fchmod()/fchown()?
why not to do something like stat() + chmod()?
(In reply to Igor Gnatenko from comment #2) > Why does it even fchmod()/fchown()? To make sure the new file has the same mode and owner as the original.
(In reply to Igor Gnatenko from comment #3) > why not to do something like stat() + chmod()? That is what it does, it stats the original file and then fchown/fchmod the new file.
This should be fixed by: commit 15973e2b9bab41a0851627863f37a877851f5610 Author: Mark Wielaard <mjw> Date: Sat Jul 21 17:20:57 2018 +0200 0.173-4 - Add elfutils-0.173-elfcompress.patch (#1607044) But also needs: commit 896e01dee76f9a08594e75ecda324fc99e03fce2 Author: Mark Wielaard <mjw> Date: Sat Jul 21 23:58:40 2018 +0200 0.173-5 - Add BuildRequires gcc-c++ for demangle support. - Add elfutils-0.173-annobingroup.patch. It still doesn't build because of a (latent?) bug on i386 handling the new annobin ELF group support in ET_REL files in eu-unstrip: https://koji.fedoraproject.org/koji/taskinfo?taskID=28503802 Other arches are fine, but this doesn't really look like it should be arch specific.
It needed another patch to eu-unstrip: commit 47092416243d54e4cff3cd2558ece8b93695d54e Author: Mark Wielaard <mark> Date: Tue Jul 24 23:34:19 2018 +0200 unstrip: Also check sh_size in compare_unalloc_sections. compare_unalloc_sections only checked sh_flags and the section names. This would cause stripped/debug section mismatches when there were multiple sections with the same name and flags. Fix this by also checking the size of the section matches. Add a testcase that has two ".group" sections created on i386 with the gcc annobin plugin. Signed-off-by: Mark Wielaard <mark> Added to elfutils-0.173-6.fc29.