From 17b4e695ce0ef89eac4a37df2df49d4c0e700766 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Wed, 24 Feb 2021 11:10:09 -0800 Subject: [PATCH] [llvm-objcopy] If input=output, preserve umask bits, otherwise drop S_ISUID/S_ISGID bits This makes the behavior similar to cp ``` chmod u+s,g+s,o+x a sudo llvm-strip a -o b // With this patch, b drops set-user-ID and set-group-ID bits. // sudo cp a b => b does not have set-user-ID or set-group-ID bits. ``` This also changes the behavior for the following case: ``` chmod u+s,g+s,o+x a llvm-strip a // a preserves set-user-ID and set-group-ID bits. // This matches binutils<2.36 and probably >=2.37. 2.36 and 2.36.1 have some compatibility issues. ``` Differential Revision: https://reviews.llvm.org/D97253 --- .../llvm-objcopy/ELF/mirror-permissions-unix.test | 12 +++++++++++ llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 24 ++++++++++------------ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test b/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test index 3995771..8f4993f 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test +++ b/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test @@ -37,12 +37,24 @@ # RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms # RUN: cmp %t1.perms %t.0640 +## Drop S_ISUID/S_ISGID bits. +# RUN: chmod 6640 %t +# RUN: llvm-objcopy %t %t1 +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms +# RUN: cmp %t1.perms %t.0640 + ## Don't set the permission of a character special file, otherwise there will ## be an EPERM error (or worse: root may change the permission). # RUN: ls -l /dev/null | cut -f 1 -d ' ' > %tnull.perms # RUN: llvm-objcopy %t /dev/null # RUN: ls -l /dev/null | cut -f 1 -d ' ' | diff - %tnull.perms +## Ignore umask if the output filename is the same as the input filename. +# RUN: umask 022 +# RUN: cp %t %t1 && chmod 0777 %t1 && llvm-objcopy %t1 +# RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms +# RUN: cmp %t1.perms %t.0777 + --- !ELF FileHeader: Class: ELFCLASS64 diff --git a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp index 42d97b2..43e3334 100644 --- a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp +++ b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp @@ -230,7 +230,7 @@ static Error executeObjcopyOnArchive(CopyConfig &Config, static Error restoreStatOnFile(StringRef Filename, const sys::fs::file_status &Stat, - bool PreserveDates) { + const CopyConfig &Config) { int FD; // Writing to stdout should not be treated as an error here, just @@ -242,7 +242,7 @@ static Error restoreStatOnFile(StringRef Filename, sys::fs::openFileForWrite(Filename, FD, sys::fs::CD_OpenExisting)) return createFileError(Filename, EC); - if (PreserveDates) + if (Config.PreserveDates) if (auto EC = sys::fs::setLastAccessAndModificationTime( FD, Stat.getLastAccessedTime(), Stat.getLastModificationTime())) return createFileError(Filename, EC); @@ -250,17 +250,17 @@ static Error restoreStatOnFile(StringRef Filename, sys::fs::file_status OStat; if (std::error_code EC = sys::fs::status(FD, OStat)) return createFileError(Filename, EC); - if (OStat.type() == sys::fs::file_type::regular_file) + if (OStat.type() == sys::fs::file_type::regular_file) { + sys::fs::perms Perm = Stat.permissions(); + if (Config.InputFilename != Config.OutputFilename) + Perm = static_cast(Perm & ~sys::fs::getUmask() & ~06000); #ifdef _WIN32 - if (auto EC = sys::fs::setPermissions( - Filename, static_cast(Stat.permissions() & - ~sys::fs::getUmask()))) + if (auto EC = sys::fs::setPermissions(Filename, Perm)) #else - if (auto EC = sys::fs::setPermissions( - FD, static_cast(Stat.permissions() & - ~sys::fs::getUmask()))) + if (auto EC = sys::fs::setPermissions(FD, Perm)) #endif return createFileError(Filename, EC); + } if (auto EC = sys::Process::SafelyCloseFileDescriptor(FD)) return createFileError(Filename, EC); @@ -320,14 +320,12 @@ static Error executeObjcopy(CopyConfig &Config) { } } - if (Error E = - restoreStatOnFile(Config.OutputFilename, Stat, Config.PreserveDates)) + if (Error E = restoreStatOnFile(Config.OutputFilename, Stat, Config)) return E; if (!Config.SplitDWO.empty()) { Stat.permissions(static_cast(0666)); - if (Error E = - restoreStatOnFile(Config.SplitDWO, Stat, Config.PreserveDates)) + if (Error E = restoreStatOnFile(Config.SplitDWO, Stat, Config)) return E; } -- 2.7.4