Avoid multiple stat() calls for same file

As a side effect we can also not use the dirent.d_type field

https://bugs.freedesktop.org/show_bug.cgi?id=68525

16 files changed, 80 insertions, 69 deletions

diff --git a/common/compat.c b/common/compat.c
index d813236..9aa556a 100644
--- a/common/compat.c
+++ b/common/compat.c
@@ -182,10 +182,11 @@ struct _p11_mmap {
 
 p11_mmap *
 p11_mmap_open (const char *path,
+               struct stat *sb,
                void **data,
                size_t *size)
 {
-	struct stat sb;
+	struct stat stb;
 	p11_mmap *map;
 
 	map = calloc (1, sizeof (p11_mmap));
@@ -198,13 +199,24 @@ p11_mmap_open (const char *path,
 		return NULL;
 	}
 
-	if (fstat (map->fd, &sb) < 0) {
+	if (sb == NULL) {
+		sb = &stb;
+		if (fstat (map->fd, &stb) < 0) {
+			close (map->fd);
+			free (map);
+			return NULL;
+		}
+	}
+
+	/* Workaround for broken ZFS on Linux */
+	if (S_ISDIR (sb->st_mode)) {
+		errno = EISDIR;
 		close (map->fd);
 		free (map);
 		return NULL;
 	}
 
-	map->size = sb.st_size;
+	map->size = sb->st_size;
 	map->data = mmap (NULL, map->size, PROT_READ, MAP_PRIVATE, map->fd, 0);
 	if (map->data == NULL) {
 		close (map->fd);
@@ -289,6 +301,7 @@ struct _p11_mmap {
 
 p11_mmap *
 p11_mmap_open (const char *path,
+               struct stat *sb,
                void **data,
                size_t *size)
 {
@@ -315,14 +328,18 @@ p11_mmap_open (const char *path,
 		return NULL;
 	}
 
-	if (!GetFileSizeEx (map->file, &large)) {
-		errn = GetLastError ();
-		CloseHandle (map->file);
-		free (map);
-		SetLastError (errn);
-		if (errn == ERROR_ACCESS_DENIED)
-			errno = EPERM;
-		return NULL;
+	if (sb == NULL) {
+		if (!GetFileSizeEx (map->file, &large)) {
+			errn = GetLastError ();
+			CloseHandle (map->file);
+			free (map);
+			SetLastError (errn);
+			if (errn == ERROR_ACCESS_DENIED)
+				errno = EPERM;
+			return NULL;
+		}
+	} else {
+		large.QuadPart = sb->st_size;
 	}
 
 	mapping = CreateFileMapping (map->file, NULL, PAGE_READONLY, 0, 0, NULL);
diff --git a/common/compat.h b/common/compat.h
index b593cf6..d7fe414 100644
--- a/common/compat.h
+++ b/common/compat.h
@@ -38,6 +38,7 @@
 #include "config.h"
 
 #include <sys/types.h>
+#include <sys/stat.h>
 
 #ifdef _GNU_SOURCE
 #error Make the crap stop. _GNU_SOURCE is completely unportable and breaks all sorts of behavior
@@ -158,6 +159,7 @@ void      p11_dl_close       (void * dl);
 typedef struct _p11_mmap p11_mmap;
 
 p11_mmap *  p11_mmap_open   (const char *path,
+                             struct stat *sb,
                              void **data,
                              size_t *size);
 
@@ -220,6 +222,7 @@ char * p11_dl_error (void);
 typedef struct _p11_mmap p11_mmap;
 
 p11_mmap *  p11_mmap_open   (const char *path,
+                             struct stat *sb,
                              void **data,
                              size_t *size);
 
diff --git a/common/test.c b/common/test.c
index daee663..83e9644 100644
--- a/common/test.c
+++ b/common/test.c
@@ -350,7 +350,7 @@ copy_file (const char *input,
 	ssize_t written;
 	size_t size;
 
-	mmap = p11_mmap_open (input, (void **)&data, &size);
+	mmap = p11_mmap_open (input, NULL, (void **)&data, &size);
 	assert (mmap != NULL);
 
 	while (size > 0) {
diff --git a/configure.ac b/configure.ac
index 13849c9..c00603c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -76,7 +76,6 @@ if test "$os_unix" = "yes"; then
 		[AC_MSG_ERROR([could not find nanosleep])])
 
 	# These are thngs we can work around
-	AC_CHECK_MEMBERS([struct dirent.d_type],,,[#include <dirent.h>])
 	AC_CHECK_FUNCS([getprogname getexecname basename mkstemp mkdtemp])
 	AC_CHECK_FUNCS([getauxval issetugid getresuid])
 	AC_CHECK_FUNCS([strnstr memdup strndup strerror_r])
diff --git a/p11-kit/conf.c b/p11-kit/conf.c
index ef542f2..8a328ed 100644
--- a/p11-kit/conf.c
+++ b/p11-kit/conf.c
@@ -92,7 +92,9 @@ _p11_conf_merge_defaults (p11_dict *map,
 }
 
 p11_dict *
-_p11_conf_parse_file (const char* filename, int flags)
+_p11_conf_parse_file (const char* filename,
+                      struct stat *sb,
+                      int flags)
 {
 	p11_dict *map = NULL;
 	void *data;
@@ -106,7 +108,7 @@ _p11_conf_parse_file (const char* filename, int flags)
 
 	p11_debug ("reading config file: %s", filename);
 
-	mmap = p11_mmap_open (filename, &data, &length);
+	mmap = p11_mmap_open (filename, sb, &data, &length);
 	if (mmap == NULL) {
 		error = errno;
 		if ((flags & CONF_IGNORE_MISSING) &&
@@ -215,7 +217,7 @@ _p11_conf_load_globals (const char *system_conf, const char *user_conf,
 	 */
 
 	/* Load the main configuration */
-	config = _p11_conf_parse_file (system_conf, CONF_IGNORE_MISSING);
+	config = _p11_conf_parse_file (system_conf, NULL, CONF_IGNORE_MISSING);
 	if (!config)
 		goto finished;
 
@@ -240,7 +242,7 @@ _p11_conf_load_globals (const char *system_conf, const char *user_conf,
 
 		/* Load up the user configuration, ignore selinux denying us access */
 		flags = CONF_IGNORE_MISSING | CONF_IGNORE_ACCESS_DENIED;
-		uconfig = _p11_conf_parse_file (path, flags);
+		uconfig = _p11_conf_parse_file (path, NULL, flags);
 		if (!uconfig) {
 			error = errno;
 			goto finished;
@@ -325,6 +327,7 @@ calc_name_from_filename (const char *fname)
 
 static bool
 load_config_from_file (const char *configfile,
+                       struct stat *sb,
                        const char *name,
                        p11_dict *configs,
                        int flags)
@@ -343,7 +346,7 @@ load_config_from_file (const char *configfile,
 		return_val_if_fail (key != NULL, false);
 	}
 
-	config = _p11_conf_parse_file (configfile, flags);
+	config = _p11_conf_parse_file (configfile, sb, flags);
 	if (!config) {
 		free (key);
 		return false;
@@ -408,22 +411,16 @@ load_configs_from_directory (const char *directory,
 		path = p11_path_build (directory, dp->d_name, NULL);
 		return_val_if_fail (path != NULL, false);
 
-#ifdef HAVE_STRUCT_DIRENT_D_TYPE
-		if(dp->d_type != DT_UNKNOWN) {
-			is_dir = (dp->d_type == DT_DIR);
-		} else
-#endif
-		{
-			if (stat (path, &st) < 0) {
-				error = errno;
-				p11_message_err (error, "couldn't stat path: %s", path);
-				free (path);
-				break;
-			}
-			is_dir = S_ISDIR (st.st_mode);
+		if (stat (path, &st) < 0) {
+			error = errno;
+			p11_message_err (error, "couldn't stat path: %s", path);
+			free (path);
+			break;
 		}
 
-		if (!is_dir && !load_config_from_file (path, dp->d_name, configs, flags)) {
+		is_dir = S_ISDIR (st.st_mode);
+
+		if (!is_dir && !load_config_from_file (path, &st, dp->d_name, configs, flags)) {
 			error = errno;
 			free (path);
 			break;
diff --git a/p11-kit/conf.h b/p11-kit/conf.h
index c4d7ae2..911e650 100644
--- a/p11-kit/conf.h
+++ b/p11-kit/conf.h
@@ -54,7 +54,8 @@ bool          _p11_conf_merge_defaults       (p11_dict *config,
                                               p11_dict *defaults);
 
 /* Returns a hash of char *key -> char *value */
-p11_dict * _p11_conf_parse_file           (const char *filename,
+p11_dict *    _p11_conf_parse_file           (const char *filename,
+                                              struct stat *sb,
                                               int flags);
 
 /* Returns a hash of char *key -> char *value */
diff --git a/p11-kit/tests/test-conf.c b/p11-kit/tests/test-conf.c
index 3a94c12..dc82da8 100644
--- a/p11-kit/tests/test-conf.c
+++ b/p11-kit/tests/test-conf.c
@@ -58,7 +58,7 @@ test_parse_conf_1 (void)
 	p11_dict *map;
 	const char *value;
 
-	map = _p11_conf_parse_file (SRCDIR "/files/test-1.conf", 0);
+	map = _p11_conf_parse_file (SRCDIR "/files/test-1.conf", NULL, 0);
 	assert_ptr_not_null (map);
 
 	value = p11_dict_get (map, "key1");
@@ -81,7 +81,7 @@ test_parse_ignore_missing (void)
 {
 	p11_dict *map;
 
-	map = _p11_conf_parse_file (SRCDIR "/files/non-existant.conf", CONF_IGNORE_MISSING);
+	map = _p11_conf_parse_file (SRCDIR "/files/non-existant.conf", NULL, CONF_IGNORE_MISSING);
 	assert_ptr_not_null (map);
 
 	assert_num_eq (0, p11_dict_size (map));
@@ -94,7 +94,7 @@ test_parse_fail_missing (void)
 {
 	p11_dict *map;
 
-	map = _p11_conf_parse_file (SRCDIR "/files/non-existant.conf", 0);
+	map = _p11_conf_parse_file (SRCDIR "/files/non-existant.conf", NULL, 0);
 	assert (map == NULL);
 	assert_ptr_not_null (p11_message_last ());
 }
diff --git a/trust/anchor.c b/trust/anchor.c
index fe46416..6620211 100644
--- a/trust/anchor.c
+++ b/trust/anchor.c
@@ -184,7 +184,7 @@ anchor_store (char **files,
 	                    NULL);
 
 	for (i = 0; i < nfiles; i++) {
-		ret = p11_parse_file (parser, files[i], P11_PARSE_FLAG_ANCHOR);
+		ret = p11_parse_file (parser, files[i], NULL, P11_PARSE_FLAG_ANCHOR);
 		switch (ret) {
 		case P11_PARSE_SUCCESS:
 			break;
diff --git a/trust/parser.c b/trust/parser.c
index 54d9c15..f89092f 100644
--- a/trust/parser.c
+++ b/trust/parser.c
@@ -749,6 +749,7 @@ p11_parse_memory (p11_parser *parser,
 int
 p11_parse_file (p11_parser *parser,
                 const char *filename,
+                struct stat *sb,
                 int flags)
 {
 	p11_mmap *map;
@@ -759,7 +760,7 @@ p11_parse_file (p11_parser *parser,
 	return_val_if_fail (parser != NULL, P11_PARSE_FAILURE);
 	return_val_if_fail (filename != NULL, P11_PARSE_FAILURE);
 
-	map = p11_mmap_open (filename, &data, &size);
+	map = p11_mmap_open (filename, sb, &data, &size);
 	if (map == NULL) {
 		p11_message_err (errno, "couldn't open and map file: %s", filename);
 		return P11_PARSE_FAILURE;
diff --git a/trust/parser.h b/trust/parser.h
index 59cc378..b177844 100644
--- a/trust/parser.h
+++ b/trust/parser.h
@@ -66,6 +66,7 @@ int           p11_parse_memory     (p11_parser *parser,
 
 int           p11_parse_file       (p11_parser *parser,
                                     const char *filename,
+                                    struct stat *sb,
                                     int flags);
 
 p11_array *   p11_parser_parsed    (p11_parser *parser);
diff --git a/trust/save.c b/trust/save.c
index a549d93..6533bd1 100644
--- a/trust/save.c
+++ b/trust/save.c
@@ -512,11 +512,11 @@ cleanup_directory (const char *directory,
                    p11_dict *cache)
 {
 	struct dirent *dp;
+	struct stat st;
 	p11_dict *remove;
 	p11_dictiter iter;
 	char *path;
 	DIR *dir;
-	int skip;
 	bool ret;
 
 	/* First we load all the modules */
@@ -535,18 +535,8 @@ cleanup_directory (const char *directory,
 		if (asprintf (&path, "%s/%s", directory, dp->d_name) < 0)
 			return_val_if_reached (false);
 
-#ifdef HAVE_STRUCT_DIRENT_D_TYPE
-		if(dp->d_type != DT_UNKNOWN) {
-			skip = (dp->d_type == DT_DIR);
-		} else
-#endif
-		{
-			struct stat st;
-
-			skip = (stat (path, &st) < 0) || S_ISDIR (st.st_mode);
-		}
 
-		if (!skip) {
+		if (stat (path, &st) >= 0 && !S_ISDIR (st.st_mode)) {
 			if (!p11_dict_set (remove, path, path))
 				return_val_if_reached (false);
 		} else {
diff --git a/trust/tests/frob-cert.c b/trust/tests/frob-cert.c
index 71018bd..c1bc45c 100644
--- a/trust/tests/frob-cert.c
+++ b/trust/tests/frob-cert.c
@@ -106,7 +106,7 @@ main (int argc,
 	ret = asn1_create_element (definitions, argv[1], &cert);
 	err_if_fail (ret, "Certificate");
 
-	map = p11_mmap_open (argv[3], &data, &size);
+	map = p11_mmap_open (argv[3], NULL, &data, &size);
 	if (map == NULL) {
 		fprintf (stderr, "couldn't open file: %s\n", argv[3]);
 		return 1;
diff --git a/trust/tests/test-module.c b/trust/tests/test-module.c
index 5920076..c272a88 100644
--- a/trust/tests/test-module.c
+++ b/trust/tests/test-module.c
@@ -1108,7 +1108,7 @@ test_create_and_write (void)
 	/* The expected file name */
 	path = p11_path_build (test.directory, "yay.p11-kit", NULL);
 	p11_parser_formats (test.parser, p11_parser_format_persist, NULL);
-	ret = p11_parse_file (test.parser, path, 0);
+	ret = p11_parse_file (test.parser, path, NULL, 0);
 	assert_num_eq (ret, P11_PARSE_SUCCESS);
 	free (path);
 
@@ -1164,7 +1164,7 @@ test_modify_and_write (void)
 
 	/* The expected file name */
 	path = p11_path_build (test.directory, "yay.p11-kit", NULL);
-	ret = p11_parse_file (test.parser, path, 0);
+	ret = p11_parse_file (test.parser, path, NULL, 0);
 	assert_num_eq (ret, P11_PARSE_SUCCESS);
 	free (path);
 
diff --git a/trust/tests/test-parser.c b/trust/tests/test-parser.c
index 09ec71c..871973b 100644
--- a/trust/tests/test-parser.c
+++ b/trust/tests/test-parser.c
@@ -118,7 +118,7 @@ test_parse_der_certificate (void)
 	};
 
 	p11_parser_formats (test.parser, p11_parser_format_x509, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der", NULL,
 	                      P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -146,7 +146,7 @@ test_parse_pem_certificate (void)
 	};
 
 	p11_parser_formats (test.parser, p11_parser_format_pem, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.pem",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.pem", NULL,
 	                      P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -173,7 +173,7 @@ test_parse_p11_kit_persist (void)
 	};
 
 	p11_parser_formats (test.parser, p11_parser_format_persist, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/input/verisign-v1.p11-kit",
+	ret = p11_parse_file (test.parser, SRCDIR "/input/verisign-v1.p11-kit", NULL,
 	                      P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -226,7 +226,7 @@ test_parse_openssl_trusted (void)
 	int i;
 
 	p11_parser_formats (test.parser, p11_parser_format_pem, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3-trusted.pem",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3-trusted.pem", NULL,
 	                      P11_PARSE_FLAG_ANCHOR);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -310,7 +310,7 @@ test_parse_openssl_distrusted (void)
 	 * so we parse this as an anchor, but expect it to be blacklisted
 	 */
 	p11_parser_formats (test.parser, p11_parser_format_pem, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/distrusted.pem",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/distrusted.pem", NULL,
 	                      P11_PARSE_FLAG_ANCHOR);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -350,7 +350,7 @@ test_parse_anchor (void)
 	int ret;
 
 	p11_parser_formats (test.parser, p11_parser_format_x509, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/cacert3.der", NULL,
 	                      P11_PARSE_FLAG_ANCHOR);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -380,7 +380,7 @@ test_parse_thawte (void)
 	};
 
 	p11_parser_formats (test.parser, p11_parser_format_pem, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/thawte.pem",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/thawte.pem", NULL,
 	                      P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
@@ -401,7 +401,7 @@ test_parse_invalid_file (void)
 	p11_message_quiet ();
 
 	p11_parser_formats (test.parser, p11_parser_format_x509, NULL);
-	ret = p11_parse_file (test.parser, "/nonexistant",
+	ret = p11_parse_file (test.parser, "/nonexistant", NULL,
 	                      P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_FAILURE, ret);
 
@@ -416,7 +416,7 @@ test_parse_unrecognized (void)
 	p11_message_quiet ();
 
 	p11_parser_formats (test.parser, p11_parser_format_x509, NULL);
-	ret = p11_parse_file (test.parser, SRCDIR "/files/unrecognized-file.txt",
+	ret = p11_parse_file (test.parser, SRCDIR "/files/unrecognized-file.txt", NULL,
 	                      P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_UNRECOGNIZED, ret);
 
@@ -433,7 +433,7 @@ test_parse_no_asn1_cache (void)
 	assert_ptr_not_null (parser);
 
 	p11_parser_formats (parser, p11_parser_format_x509, NULL);
-	ret = p11_parse_file (parser, SRCDIR "/files/cacert3.der", P11_PARSE_FLAG_NONE);
+	ret = p11_parse_file (parser, SRCDIR "/files/cacert3.der", NULL, P11_PARSE_FLAG_NONE);
 	assert_num_eq (P11_PARSE_SUCCESS, ret);
 
 	/* Should have gotten certificate  */
diff --git a/trust/tests/test-token.c b/trust/tests/test-token.c
index bdf1120..a028d9c 100644
--- a/trust/tests/test-token.c
+++ b/trust/tests/test-token.c
@@ -238,9 +238,11 @@ test_not_writable (void)
 {
 	p11_token *token;
 
-	token = p11_token_new (333, "/", "Label");
-	assert (!p11_token_is_writable (token));
-	p11_token_free (token);
+	if (getuid () != 0) {
+		token = p11_token_new (333, "/", "Label");
+		assert (!p11_token_is_writable (token));
+		p11_token_free (token);
+	}
 
 	token = p11_token_new (333, "", "Label");
 	assert (!p11_token_is_writable (token));
@@ -533,7 +535,7 @@ test_write_new (void)
 
 	/* The expected file name */
 	path = p11_path_build (test.directory, "Yay_.p11-kit", NULL);
-	ret = p11_parse_file (test.parser, path, 0);
+	ret = p11_parse_file (test.parser, path, NULL, 0);
 	assert_num_eq (ret, P11_PARSE_SUCCESS);
 	free (path);
 
@@ -573,7 +575,7 @@ test_write_no_label (void)
 
 	/* The expected file name */
 	path = p11_path_build (test.directory, "data.p11-kit", NULL);
-	ret = p11_parse_file (test.parser, path, 0);
+	ret = p11_parse_file (test.parser, path, NULL, 0);
 	assert_num_eq (ret, P11_PARSE_SUCCESS);
 	free (path);
 
diff --git a/trust/token.c b/trust/token.c
index 4e7c631..22363f8 100644
--- a/trust/token.c
+++ b/trust/token.c
@@ -179,7 +179,7 @@ loader_load_file (p11_token *token,
 	else if (strcmp (filename, token->path) == 0 && !S_ISDIR (sb->st_mode))
 		flags = P11_PARSE_FLAG_ANCHOR;
 
-	ret = p11_parse_file (token->parser, filename, flags);
+	ret = p11_parse_file (token->parser, filename, sb, flags);
 
 	switch (ret) {
 	case P11_PARSE_SUCCESS: