Browse Source

Merge topic 'rpath-preserve-setuid-setgid'

0907a322f3 install: Restore SETUID/SETGID after RPATH change

Acked-by: Kitware Robot <[email protected]>
Merge-request: !10053
Brad King 10 months ago
parent
commit
79e41d3cc6
2 changed files with 141 additions and 7 deletions
  1. 111 7
      Source/cmSystemTools.cxx
  2. 30 0
      Tests/RunCMake/file-RPATH/Common.cmake

+ 111 - 7
Source/cmSystemTools.cxx

@@ -1186,6 +1186,76 @@ std::string cmSystemTools::GetComspec()
 
 #endif
 
+// File changes involve removing SETUID/SETGID bits when a file is modified.
+// This behavior is consistent across most Unix-like operating systems.
+class FileModeGuard
+{
+public:
+  FileModeGuard(const std::string& file_path, std::string* emsg);
+  bool Restore(std::string* emsg);
+  bool HasErrors() const;
+
+private:
+#ifndef _WIN32
+  mode_t mode_;
+#endif
+  std::string filepath_;
+};
+
+FileModeGuard::FileModeGuard(const std::string& file_path, std::string* emsg)
+{
+#ifndef _WIN32
+  struct stat file_stat;
+  if (stat(file_path.c_str(), &file_stat) != 0) {
+    if (emsg) {
+      *emsg = cmStrCat("Cannot get file stat: ", strerror(errno));
+    }
+    return;
+  }
+
+  mode_ = file_stat.st_mode;
+#else
+  static_cast<void>(emsg);
+#endif
+  filepath_ = file_path;
+}
+
+bool FileModeGuard::Restore(std::string* emsg)
+{
+  assert(filepath_.empty() == false);
+
+#ifndef _WIN32
+  struct stat file_stat;
+  if (stat(filepath_.c_str(), &file_stat) != 0) {
+    if (emsg) {
+      *emsg = cmStrCat("Cannot get file stat: ", strerror(errno));
+    }
+    return false;
+  }
+
+  // Nothing changed; everything is in the expected state
+  if (file_stat.st_mode == mode_) {
+    return true;
+  }
+
+  if (chmod(filepath_.c_str(), mode_) != 0) {
+    if (emsg) {
+      *emsg = cmStrCat("Cannot restore the file mode: ", strerror(errno));
+    }
+    return false;
+  }
+#else
+  static_cast<void>(emsg);
+#endif
+
+  return true;
+}
+
+bool FileModeGuard::HasErrors() const
+{
+  return filepath_.empty();
+}
+
 std::string cmSystemTools::GetRealPath(const std::string& path,
                                        std::string* errorMessage)
 {
@@ -3256,6 +3326,11 @@ cm::optional<bool> AdjustRPathELF(std::string const& file,
     return cmSystemTools::RemoveRPath(file, emsg, changed);
   }
 
+  FileModeGuard file_mode_guard(file, emsg);
+  if (file_mode_guard.HasErrors()) {
+    return false;
+  }
+
   {
     // Open the file for update.
     cmsys::ofstream f(file.c_str(),
@@ -3295,6 +3370,10 @@ cm::optional<bool> AdjustRPathELF(std::string const& file,
     }
   }
 
+  if (!file_mode_guard.Restore(emsg)) {
+    return false;
+  }
+
   // Everything was updated successfully.
   if (changed) {
     *changed = true;
@@ -3788,6 +3867,11 @@ static cm::optional<bool> RemoveRPathELF(std::string const& file,
     bytesBegin = elf.GetDynamicEntryPosition(0);
   }
 
+  FileModeGuard file_mode_guard(file, emsg);
+  if (file_mode_guard.HasErrors()) {
+    return false;
+  }
+
   // Open the file for update.
   cmsys::ofstream f(file.c_str(),
                     std::ios::in | std::ios::out | std::ios::binary);
@@ -3831,6 +3915,13 @@ static cm::optional<bool> RemoveRPathELF(std::string const& file,
     }
   }
 
+  // Close the handle to allow further operations on the file
+  f.close();
+
+  if (!file_mode_guard.Restore(emsg)) {
+    return false;
+  }
+
   // Everything was updated successfully.
   if (removed) {
     *removed = true;
@@ -3849,15 +3940,28 @@ static cm::optional<bool> RemoveRPathXCOFF(std::string const& file,
   (void)emsg;
   return cm::nullopt; // Cannot handle XCOFF files.
 #else
-  cmXCOFF xcoff(file.c_str(), cmXCOFF::Mode::ReadWrite);
-  if (!xcoff) {
-    return cm::nullopt; // Not a valid XCOFF file.
+  bool rm = false;
+
+  FileModeGuard file_mode_guard(file, emsg);
+  if (file_mode_guard.HasErrors()) {
+    return false;
   }
-  bool rm = xcoff.RemoveLibPath();
-  if (!xcoff) {
-    if (emsg) {
-      *emsg = xcoff.GetErrorMessage();
+
+  {
+    cmXCOFF xcoff(file.c_str(), cmXCOFF::Mode::ReadWrite);
+    if (!xcoff) {
+      return cm::nullopt; // Not a valid XCOFF file.
+    }
+    rm = xcoff.RemoveLibPath();
+    if (!xcoff) {
+      if (emsg) {
+        *emsg = xcoff.GetErrorMessage();
+      }
+      return false;
     }
+  }
+
+  if (!file_mode_guard.Restore(emsg)) {
     return false;
   }
 

+ 30 - 0
Tests/RunCMake/file-RPATH/Common.cmake

@@ -1,3 +1,26 @@
+if(CMAKE_HOST_UNIX)
+  function(permissions_set f)
+    file(CHMOD ${f} FILE_PERMISSIONS OWNER_WRITE OWNER_READ OWNER_EXECUTE SETUID)
+  endfunction()
+  function(permissions_check f)
+    execute_process(COMMAND sh -c "stat -c %a \"${f}\""
+      OUTPUT_VARIABLE stat_out OUTPUT_STRIP_TRAILING_WHITESPACE
+      ERROR_VARIABLE stat_err
+      RESULT_VARIABLE stat_res
+      )
+    if(stat_res EQUAL 0)
+      if(NOT stat_out STREQUAL "4700")
+        message(FATAL_ERROR "Expected permissions 4700 but got ${stat_out}:\n ${f}")
+      endif()
+    endif()
+  endfunction()
+else()
+  function(permissions_set)
+  endfunction()
+  function(permissions_check)
+  endfunction()
+endif()
+
 # Prepare binaries on which to operate.
 set(in "${CMAKE_CURRENT_LIST_DIR}/${format}")
 set(out "${CMAKE_CURRENT_BINARY_DIR}")
@@ -11,11 +34,14 @@ foreach(f ${static})
 endforeach()
 
 foreach(f ${dynamic_files})
+  permissions_set(${f})
+
   # Check for the initial RPATH.
   file(RPATH_CHECK FILE "${f}" RPATH "/sample/rpath")
   if(NOT EXISTS "${f}")
     message(FATAL_ERROR "RPATH_CHECK removed ${f}")
   endif()
+  permissions_check(${f})
 
   # Change the RPATH.
   file(RPATH_CHANGE FILE "${f}"
@@ -26,6 +52,7 @@ foreach(f ${dynamic_files})
   if(NOT rpath)
     message(FATAL_ERROR "RPATH not changed in ${f}")
   endif()
+  permissions_check(${f})
 
   # Change the RPATH without compiler defined rpath removed
   file(RPATH_CHANGE FILE "${f}"
@@ -36,6 +63,7 @@ foreach(f ${dynamic_files})
   if(NOT rpath)
     message(FATAL_ERROR "RPATH not updated in ${f}")
   endif()
+  permissions_check(${f})
 
   # Change the RPATH with compiler defined rpath removed
   file(RPATH_CHANGE FILE "${f}"
@@ -51,6 +79,7 @@ foreach(f ${dynamic_files})
   if(rpath)
     message(FATAL_ERROR "RPATH not removed in ${f}")
   endif()
+  permissions_check(${f})
 
   # Remove the RPATH.
   file(RPATH_REMOVE FILE "${f}")
@@ -59,6 +88,7 @@ foreach(f ${dynamic_files})
   if(rpath)
     message(FATAL_ERROR "RPATH not removed from ${f}")
   endif()
+  permissions_check(${f})
 
   # Check again...this should remove the file.
   file(RPATH_CHECK FILE "${f}" RPATH "/sample/rpath")