fix: Add and use Util::{get,set}_umask functions

Using

    mode_t mask = umask(0);
    umask(mask);

to retrieve the current umask isn't thread-safe. This messed up file
permissions when recompressing the cache.

Fix this by caching the last set value and always using
Util::{get,set}_umask to get and set the umask.

diff --git a/src/Context.cpp b/src/Context.cpp
index e8dc80d734976cdb4138d00bcb3eb5874d6b8cd8..3b0628c287b29c797cd8a4d6071222a9716b65c7 100644 (file)
--- a/src/Context.cpp
+++ b/src/Context.cpp
@@ -62,7 +62,7 @@ Context::initialize()
   // in the cache directory should be affected by the configured umask and that
   // no other files and directories should.
   if (config.umask()) {
-    original_umask = umask(*config.umask());
+    original_umask = Util::set_umask(*config.umask());
   }
 }
 

diff --git a/src/UmaskScope.hpp b/src/UmaskScope.hpp
index e71eb0a7507491e15747a6cdf75eead274763568..02bf8e8cbf562201950b8e6fbd0b0080cf2b717e 100644 (file)
--- a/src/UmaskScope.hpp
+++ b/src/UmaskScope.hpp
@@ -39,7 +39,7 @@ inline UmaskScope::UmaskScope(std::optional<mode_t> new_umask)
 {
 #ifndef _WIN32
   if (new_umask) {
-    m_saved_umask = umask(*new_umask);
+    m_saved_umask = Util::set_umask(*new_umask);
   }
 #else
   (void)new_umask;
@@ -55,7 +55,7 @@ inline UmaskScope::~UmaskScope()
 #    pragma GCC diagnostic push
 #    pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
 #  endif
-    umask(*m_saved_umask);
+    Util::set_umask(*m_saved_umask);
 #  if defined(__GNUC__) && !defined(__clang__)
 #    pragma GCC diagnostic pop
 #  endif

diff --git a/src/Util.cpp b/src/Util.cpp
index 7770fabd48f433f7e64f4fd9a5138adc84c515ec..c2ce61dca9dff11355a5f672e4589a4473b7f3f3 100644 (file)
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -98,6 +98,13 @@ using IncludeDelimiter = util::Tokenizer::IncludeDelimiter;
 
 namespace {
 
+// Process umask, read and written by get_umask and set_umask.
+mode_t g_umask = [] {
+  const mode_t mask = umask(0);
+  umask(mask);
+  return mask;
+}();
+
 // Search for the first match of the following regular expression:
 //
 //   \x1b\[[\x30-\x3f]*[\x20-\x2f]*[Km]
@@ -741,9 +748,7 @@ get_relative_path(std::string_view dir, std::string_view path)
 mode_t
 get_umask()
 {
-  const mode_t mask = umask(0);
-  umask(mask);
-  return mask;
+  return g_umask;
 }
 
 void
@@ -1295,6 +1300,13 @@ set_cloexec_flag(int fd)
 #endif
 }
 
+mode_t
+set_umask(mode_t mask)
+{
+  g_umask = mask;
+  return umask(mask);
+}
+
 void
 setenv(const std::string& name, const std::string& value)
 {

diff --git a/src/Util.hpp b/src/Util.hpp
index 0cf84567745103a26075a02ba2e9689469d68af9..c8231107f6e6f8f49ef3e4e573b8533abfbccc39 100644 (file)
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -341,6 +341,9 @@ void send_to_fd(const Context& ctx, const std::string& text, int fd);
 // Set the FD_CLOEXEC on file descriptor `fd`. This is a NOP on Windows.
 void set_cloexec_flag(int fd);
 
+// Set process umask. Returns the previous mask.
+mode_t set_umask(mode_t mask);
+
 // Set environment variable `name` to `value`.
 void setenv(const std::string& name, const std::string& value);
 

diff --git a/src/ccache.cpp b/src/ccache.cpp
index c6ad63ec1289c392a4478261b08a3d9adb9d3d80..f17631355be166641559644f6e6f4d3fc524281d 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -2112,7 +2112,7 @@ cache_compilation(int argc, const char* const* argv)
 
   if (fall_back_to_original_compiler) {
     if (original_umask) {
-      umask(*original_umask);
+      Util::set_umask(*original_umask);
     }
     auto execv_argv = saved_orig_args.to_argv();
     execute_noreturn(execv_argv.data(), saved_temp_dir);