llvm.org GIT mirror llvm / 2a913c7
[llvm-objcopy] Don't change permissions of non-regular output files There is currently an EPERM error when a regular user executes `llvm-objcopy a.o /dev/null`. Worse, root can even change the mode bits of /dev/null. Fix it by checking if the output file is special. A new overload of llvm::sys::fs::setPermissions with FD as the parameter is added. Users should provide `perm & ~umask` as the parameter if they intend to respect umask. The existing overload of llvm::sys::fs::setPermissions may be deleted if we can find an implementation of fchmod() on Windows. fchmod() is usually better than chmod() because it saves syscalls and can avoid race condition. Reviewed By: jakehehrlich, jhenderson Differential Revision: https://reviews.llvm.org/D64236 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365753 91177308-0d34-0410-b5e6-96231b3b80d8 Fangrui Song 3 months ago
6 changed file(s) with 44 addition(s) and 17 deletion(s). Raw diff Collapse all Expand all
664664 ///
665665 /// @param Path File to set permissions on.
666666 /// @param Permissions New file permissions.
667 /// @param RespectUmask If true then Permissions will be changed to respect the
668 /// umask of the current process.
669667 /// @returns errc::success if the permissions were successfully set, otherwise
670668 /// a platform-specific error_code.
671669 /// @note On Windows, all permissions except *_write are ignored. Using any of
672670 /// owner_write, group_write, or all_write will make the file writable.
673671 /// Otherwise, the file will be marked as read-only.
674 std::error_code setPermissions(const Twine &Path, perms Permissions,
675 bool RespectUmask = false);
672 std::error_code setPermissions(const Twine &Path, perms Permissions);
673
674 /// Vesion of setPermissions accepting a file descriptor.
675 /// TODO Delete the path based overload once we implement the FD based overload
676 /// on Windows.
677 std::error_code setPermissions(int FD, perms Permissions);
676678
677679 /// Get file permissions.
678680 ///
701701 return Mask;
702702 }
703703
704 std::error_code setPermissions(const Twine &Path, perms Permissions,
705 bool RespectUmask) {
704 std::error_code setPermissions(const Twine &Path, perms Permissions) {
706705 SmallString<128> PathStorage;
707706 StringRef P = Path.toNullTerminatedStringRef(PathStorage);
708707
709 if (RespectUmask)
710 Permissions = static_cast(Permissions & ~getUmask());
711
712708 if (::chmod(P.begin(), Permissions))
709 return std::error_code(errno, std::generic_category());
710 return std::error_code();
711 }
712
713 std::error_code setPermissions(int FD, perms Permissions) {
714 if (::fchmod(FD, Permissions))
713715 return std::error_code(errno, std::generic_category());
714716 return std::error_code();
715717 }
741741 return 0;
742742 }
743743
744 std::error_code setPermissions(const Twine &Path, perms Permissions,
745 bool /*RespectUmask*/) {
744 std::error_code setPermissions(const Twine &Path, perms Permissions) {
746745 SmallVector PathUTF16;
747746 if (std::error_code EC = widenPath(Path, PathUTF16))
748747 return EC;
771770 return mapWindowsError(GetLastError());
772771
773772 return std::error_code();
773 }
774
775 std::error_code setPermissions(int FD, perms Permissions) {
776 // FIXME Not implemented.
777 return std::make_error_code(std::errc::not_supported);
774778 }
775779
776780 std::error_code setLastAccessAndModificationTime(int FD, TimePoint<> AccessTime,
3535 # RUN: ls -l %t1 | cut -f 1 -d ' ' > %t1.perms
3636 # RUN: cmp %t1.perms %t.0640
3737
38 ## Don't set the permission of a character special file, otherwise there will
39 ## be an EPERM error (or worse: root may change the permission).
40 # RUN: ls -l /dev/null | cut -f 1 -d ' ' > %tnull.perms
41 # RUN: llvm-objcopy %t /dev/null
42 # RUN: ls -l /dev/null | cut -f 1 -d ' ' | diff - %tnull.perms
43
3844 --- !ELF
3945 FileHeader:
4046 Class: ELFCLASS64
221221 FD, Stat.getLastAccessedTime(), Stat.getLastModificationTime()))
222222 return createFileError(Filename, EC);
223223
224 if (auto EC = sys::fs::setPermissions(Filename, Stat.permissions(),
225 /*respectUmask=*/true))
224 sys::fs::file_status OStat;
225 if (std::error_code EC = sys::fs::status(FD, OStat))
226226 return createFileError(Filename, EC);
227 if (OStat.type() == sys::fs::file_type::regular_file)
228 #ifdef _WIN32
229 if (auto EC = sys::fs::setPermissions(
230 Filename, static_cast(Stat.permissions() &
231 ~sys::fs::getUmask())))
232 #else
233 if (auto EC = sys::fs::setPermissions(
234 FD, static_cast(Stat.permissions() &
235 ~sys::fs::getUmask())))
236 #endif
237 return createFileError(Filename, EC);
227238
228239 if (auto EC = sys::Process::SafelyCloseFileDescriptor(FD))
229240 return createFileError(Filename, EC);
15491549
15501550 fs::perms AllRWE = static_cast(0777);
15511551
1552 ASSERT_NO_ERROR(fs::setPermissions(TempPath, AllRWE /*RespectUmask=false*/));
1552 ASSERT_NO_ERROR(fs::setPermissions(TempPath, AllRWE));
15531553
15541554 ErrorOr Perms = fs::getPermissions(TempPath);
15551555 ASSERT_TRUE(!!Perms);
15561556 EXPECT_EQ(Perms.get(), AllRWE) << "Should have ignored umask by default";
15571557
1558 ASSERT_NO_ERROR(fs::setPermissions(TempPath, AllRWE, /*RespectUmask=*/false));
1558 ASSERT_NO_ERROR(fs::setPermissions(TempPath, AllRWE));
15591559
15601560 Perms = fs::getPermissions(TempPath);
15611561 ASSERT_TRUE(!!Perms);
15621562 EXPECT_EQ(Perms.get(), AllRWE) << "Should have ignored umask";
15631563
1564 ASSERT_NO_ERROR(fs::setPermissions(TempPath, AllRWE, /*RespectUmask=*/true));
1564 ASSERT_NO_ERROR(
1565 fs::setPermissions(FD, static_cast(AllRWE & ~fs::getUmask())));
15651566 Perms = fs::getPermissions(TempPath);
15661567 ASSERT_TRUE(!!Perms);
15671568 EXPECT_EQ(Perms.get(), static_cast(0755))
15691570
15701571 (void)::umask(0057);
15711572
1572 ASSERT_NO_ERROR(fs::setPermissions(TempPath, AllRWE, /*RespectUmask=*/true));
1573 ASSERT_NO_ERROR(
1574 fs::setPermissions(FD, static_cast(AllRWE & ~fs::getUmask())));
15731575 Perms = fs::getPermissions(TempPath);
15741576 ASSERT_TRUE(!!Perms);
15751577 EXPECT_EQ(Perms.get(), static_cast(0720))