Merge branch '3.4-maint'

* 3.4-maint: Update NEWS Avoid TOCTOU issue when deciding if config is valid
ccache · May 7, 2018 · 13377bb · 13377bb
2 parents a630d2e + c90c1ef
commit 13377bb
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 2 deletions.
diff --git a/doc/NEWS.adoc b/doc/NEWS.adoc
@@ -23,6 +23,17 @@ New features and enhancements
   so updates to the file will now correctly result in separate cache entries.
 
 
+ccache 3.4.3
+-----------
+Release date: unknown
+
+Bug fixes
+~~~~~~~~~
+
+- Fixed a race condition when creating the initial config file in the cache
+  directory.
+
+
 ccache 3.4.2
 ------------
 Release date: 2018-03-25

diff --git a/src/ccache.c b/src/ccache.c
@@ -3157,7 +3157,7 @@ initialize(void)
 	} else {
 		secondary_config_path = format("%s/ccache.conf", TO_STRING(SYSCONFDIR));
 		if (!conf_read(conf, secondary_config_path, &errmsg)) {
-			if (access(secondary_config_path, R_OK) == 0) {
+			if (errno == 0) {
 				// We could read the file but it contained errors.
 				fatal("%s", errmsg);
 			}
@@ -3181,7 +3181,7 @@ initialize(void)
 
 	bool should_create_initial_config = false;
 	if (!conf_read(conf, primary_config_path, &errmsg)) {
-		if (access(primary_config_path, R_OK) == 0) {
+		if (errno == 0) {
 			// We could read the file but it contained errors.
 			fatal("%s", errmsg);
 		}

diff --git a/src/conf.c b/src/conf.c
@@ -382,6 +382,9 @@ conf_free(struct conf *conf)
 }
 
 // Note: The path pointer is stored in conf, so path must outlive conf.
+//
+// On failure, if an I/O error occured errno is set approriately, otherwise
+// errno is set to zero indicating that config itself was invalid.
 bool
 conf_read(struct conf *conf, const char *path, char **errmsg)
 {
@@ -412,6 +415,7 @@ conf_read(struct conf *conf, const char *path, char **errmsg)
 		if (!ok) {
 			*errmsg = format("%s:%u: %s", path, line_number, errmsg2);
 			free(errmsg2);
+			errno = 0;
 			result = false;
 			goto out;
 		}

diff --git a/unittest/test_conf.c b/unittest/test_conf.c
@@ -186,6 +186,7 @@ TEST(conf_read_with_missing_equal_sign)
 	char *errmsg;
 	create_file("ccache.conf", "no equal sign");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: missing equal sign",
 	                   errmsg);
 	conf_free(conf);
@@ -197,6 +198,7 @@ TEST(conf_read_with_bad_config_key)
 	char *errmsg;
 	create_file("ccache.conf", "# Comment\nfoo = bar");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:2: unknown configuration option \"foo\"",
 	                   errmsg);
 	conf_free(conf);
@@ -209,11 +211,13 @@ TEST(conf_read_invalid_bool)
 
 	create_file("ccache.conf", "disable=");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: not a boolean value: \"\"",
 	                   errmsg);
 
 	create_file("ccache.conf", "disable=foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: not a boolean value: \"foo\"",
 	                   errmsg);
 	conf_free(conf);
@@ -225,6 +229,7 @@ TEST(conf_read_invalid_env_string)
 	char *errmsg;
 	create_file("ccache.conf", "base_dir = ${foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: syntax error: missing '}' after \"foo\"",
 	                   errmsg);
 	// Other cases tested in test_util.c.
@@ -247,6 +252,7 @@ TEST(conf_read_invalid_size)
 	char *errmsg;
 	create_file("ccache.conf", "max_size = foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: invalid size: \"foo\"",
 	                   errmsg);
 	// Other cases tested in test_util.c.
@@ -259,6 +265,7 @@ TEST(conf_read_invalid_sloppiness)
 	char *errmsg;
 	create_file("ccache.conf", "sloppiness = file_macro, foo");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: unknown sloppiness: \"foo\"",
 	                   errmsg);
 	conf_free(conf);
@@ -271,6 +278,7 @@ TEST(conf_read_invalid_unsigned)
 
 	create_file("ccache.conf", "max_files =");
 	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, 0);
 	CHECK_STR_EQ_FREE2("ccache.conf:1: invalid unsigned integer: \"\"",
 	                   errmsg);
 
@@ -287,6 +295,14 @@ TEST(conf_read_invalid_unsigned)
 	conf_free(conf);
 }
 
+TEST(conf_read_missing_config_file)
+{
+	struct conf *conf = conf_create();
+	char *errmsg;
+	CHECK(!conf_read(conf, "ccache.conf", &errmsg));
+	CHECK_INT_EQ(errno, ENOENT);
+}
+
 TEST(verify_absolute_base_dir)
 {
 	struct conf *conf = conf_create();