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.

(cherry picked from commit f8f1c35344e37bc52c9fe71daebb5b378bd353f3)

diff --git a/src/Context.cpp b/src/Context.cpp
index bcfff8c78a46d97b4bc21340510ebc7ed4f008bf..c82169b20f5c8ac1a5a2cc7b8b1a8041aca44afc 100644 (file)
--- a/src/Context.cpp
+++ b/src/Context.cpp
@@ -64,7 +64,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 2a2f85a74425591a339d5c4412663dc4bd8b9a5b..58d9e009693e8b9d9bcab990fc41e697a08b6c8f 100644 (file)
--- a/src/UmaskScope.hpp
+++ b/src/UmaskScope.hpp
@@ -39,7 +39,7 @@ inline UmaskScope::UmaskScope(nonstd::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;
@@ -50,7 +50,7 @@ inline UmaskScope::~UmaskScope()
 {
 #ifndef _WIN32
   if (m_saved_umask) {
-    umask(*m_saved_umask);
+    Util::set_umask(*m_saved_umask);
   }
 #endif
 }

diff --git a/src/Util.cpp b/src/Util.cpp
index 6bbab611d88774ce689fe998b837af6b0cd6c96e..df6835a4a357bdce25521587023b843bd1ba5d13 100644 (file)
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -101,6 +101,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]
@@ -744,9 +751,7 @@ get_relative_path(string_view dir, string_view path)
 mode_t
 get_umask()
 {
-  const mode_t mask = umask(0);
-  umask(mask);
-  return mask;
+  return g_umask;
 }
 
 void
@@ -1296,6 +1301,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 092f9aaafa82a537a4699c6de72a15280df49cb9..85d1a2663153ba7b1a44c5435a87573a66cbf5c7 100644 (file)
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -345,6 +345,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 23d282922cd26b4b88cb6ebac0e969731de58285..4f691323cd7990a5012688bd6f1893457c037936 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -2090,7 +2090,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);