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

diff --git a/common/compat.c b/common/compat.c
index d81323629de05e5fd34fdf4b17082569d85cab78..9aa556a4f6c09d752d30f61227ed35b305103977 100644 (file)
--- 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 b593cf607ce3b3aad9e4dc6e0259091b0fd2c870..d7fe414a46bf0c49c677d8c97750df62db0f7059 100644 (file)
--- 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 daee663f655cc6f85c377db4c9af70cfb7b9e839..83e9644270b2a8c7dc86023bef88ea50d0d8ce93 100644 (file)
--- 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 13849c9d5705201364ea3e4d275f6297c66225fa..c00603c07273dc9aea9324c65f6552f422f72a66 100644 (file)
--- 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 ef542f29d7341f5eb398992c0810882dcd45d2fe..8a328ed30f073cd975a12093f4d6585c3d7ba2c8 100644 (file)
--- 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 c4d7ae21b9ba2c3d28ea98ba584ab3ae6fdadfb7..911e650972738e8d1525286fdacae23bb616ec6a 100644 (file)
--- 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 3a94c125badd7eb87dc077ffc279ffb0930105d5..dc82da8679474fa24a095ec33981efd3d3c8e4ee 100644 (file)
--- 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 fe4641650f73333b58e5383e7a4b0024547e66ee..6620211fd997d295d924d8d7e52976c899bcb901 100644 (file)
--- 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 54d9c15418b6d360cc3c841b6f819192101e866a..f89092f7168150645acc1493de958a6a70c077a4 100644 (file)
--- 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 59cc378c23bd432b45d0c8f5ce0d4b1c058fd384..b1778447ab10be59e5da1288c46cd613fef16d7b 100644 (file)
--- 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 a549d936d171d557c05c861ba7e0a0dfd1a60424..6533bd19caed4cede155373c109c1c32a184fc36 100644 (file)
--- 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 71018bd4de37cae0120e5ea452470639c8c6f541..c1bc45c5f7a74d23c6ad43452680a36fe0032cda 100644 (file)
--- 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 59200761388635323b445c5d585cc80bd7db77bf..c272a8883fc95621229a4da01d9bbf9b12035a35 100644 (file)
--- 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 09ec71ce6f2af5f834d1ab026f6b566312b402c7..871973bb351569f2922ac4fc405bef5cb550060b 100644 (file)
--- 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 bdf1120b3195e51b8af80ed7d082e6685f69be87..a028d9c05b9778a7ce8ba1b5063a3a06c99380c6 100644 (file)
--- 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 4e7c63167255d32b1aaaefa298a1c9e0117b03c9..22363f8d395d3c0a108493e52d1fb47a5199e5da 100644 (file)
--- 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: