Skip to content

Commit

Permalink
[llvm-objcopy] If input=output, preserve umask bits, otherwise drop S…
Browse files Browse the repository at this point in the history
…_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
  • Loading branch information
MaskRay committed Feb 24, 2021
1 parent c2487bf commit 17b4e69
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
12 changes: 12 additions & 0 deletions llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 11 additions & 13 deletions llvm/tools/llvm-objcopy/llvm-objcopy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -242,25 +242,25 @@ 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);

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<sys::fs::perms>(Perm & ~sys::fs::getUmask() & ~06000);
#ifdef _WIN32
if (auto EC = sys::fs::setPermissions(
Filename, static_cast<sys::fs::perms>(Stat.permissions() &
~sys::fs::getUmask())))
if (auto EC = sys::fs::setPermissions(Filename, Perm))
#else
if (auto EC = sys::fs::setPermissions(
FD, static_cast<sys::fs::perms>(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);
Expand Down Expand Up @@ -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<sys::fs::perms>(0666));
if (Error E =
restoreStatOnFile(Config.SplitDWO, Stat, Config.PreserveDates))
if (Error E = restoreStatOnFile(Config.SplitDWO, Stat, Config))
return E;
}

Expand Down

0 comments on commit 17b4e69

Please sign in to comment.