]> granicus.if.org Git - p11-kit/commitdiff
open files with O_CLOEXEC when possible
authorStef Walter <stef@thewalter.net>
Tue, 16 Jul 2013 20:43:37 +0000 (22:43 +0200)
committerStef Walter <stef@thewalter.net>
Thu, 18 Jul 2013 05:33:57 +0000 (07:33 +0200)
This helps prevent leaked file descriptors when the library is
used in a process which exec's.

opendir() already uses O_CLOEXEC on platforms that support O_CLOEXEC
so we don't need to make changes there.

In addition read config files using p11_mmap_open() so that we get
the simple benefits of O_CLOEXEC with the open() call there.

https://bugzilla.redhat.com/show_bug.cgi?id=984986

common/compat.c
common/compat.h
p11-kit/conf.c
p11-kit/pin.c

index 400e10bb0da870804cc5e184f808091954618e66..5efc93214aa295c0c7394fa0ff4b0c43cc8b67ab 100644 (file)
@@ -192,7 +192,7 @@ p11_mmap_open (const char *path,
        if (map == NULL)
                return NULL;
 
-       map->fd = open (path, O_RDONLY);
+       map->fd = open (path, O_RDONLY | O_CLOEXEC);
        if (map->fd == -1) {
                free (map);
                return NULL;
@@ -298,14 +298,20 @@ p11_mmap_open (const char *path,
        p11_mmap *map;
 
        map = calloc (1, sizeof (p11_mmap));
-       if (map == NULL)
+       if (map == NULL) {
+               errno = ENOMEM;
                return NULL;
+       }
 
        map->file  = CreateFile (path, GENERIC_READ, 0, NULL, OPEN_EXISTING, FILE_FLAG_RANDOM_ACCESS, NULL);
        if (map->file == INVALID_HANDLE_VALUE) {
                errn = GetLastError ();
                free (map);
                SetLastError (errn);
+               if (errn == ERROR_PATH_NOT_FOUND || errn == ERROR_FILE_NOT_FOUND)
+                       errno = ENOENT;
+               else if (errn == ERROR_ACCESS_DENIED)
+                       errno = EPERM;
                return NULL;
        }
 
@@ -314,6 +320,8 @@ p11_mmap_open (const char *path,
                CloseHandle (map->file);
                free (map);
                SetLastError (errn);
+               if (errn == ERROR_ACCESS_DENIED)
+                       errno = EPERM;
                return NULL;
        }
 
@@ -323,6 +331,8 @@ p11_mmap_open (const char *path,
                CloseHandle (map->file);
                free (map);
                SetLastError (errn);
+               if (errn == ERROR_ACCESS_DENIED)
+                       errno = EPERM;
                return NULL;
        }
 
@@ -334,6 +344,8 @@ p11_mmap_open (const char *path,
                CloseHandle (map->file);
                free (map);
                SetLastError (errn);
+               if (errn == ERROR_ACCESS_DENIED)
+                       errno = EPERM;
                return NULL;
        }
 
@@ -676,7 +688,7 @@ _gettemp (char *path,
 
        for (;;) {
                if (doopen) {
-                       if ((*doopen = open (path, O_BINARY | O_CREAT | O_EXCL | O_RDWR, 0600)) >= 0)
+                       if ((*doopen = open (path, O_BINARY | O_CREAT | O_EXCL | O_RDWR | O_CLOEXEC, 0600)) >= 0)
                                return (1);
                        if (errno != EEXIST)
                                return (0);
index 9127f95988bf75434d6dda0754c4f20c0fe75c90..20f9a8109c0a8a19e97dd42a828b0bfa23e289a0 100644 (file)
 #define O_BINARY 0
 #endif
 
+#ifndef O_CLOEXEC
+#define O_CLOEXEC 0
+#endif
+
 #ifndef HAVE_GETPROGNAME
 const char * getprogname (void);
 #endif
index 2138d3e0f03228ccfcd3494fa80cee3faadfbdcd..e699e66ab7c6329c418ef467dc8b14b716dc80ab 100644 (file)
@@ -67,76 +67,6 @@ strequal (const char *one, const char *two)
  * CONFIG PARSER
  */
 
-static char *
-read_config_file (const char* filename,
-                  int flags,
-                  size_t *length)
-{
-       char* config = NULL;
-       FILE* f = NULL;
-       int error = 0;
-       long len;
-
-       assert (filename);
-
-       f = fopen (filename, "rb");
-       if (f == NULL) {
-               error = errno;
-               if ((flags & CONF_IGNORE_MISSING) &&
-                   (error == ENOENT || error == ENOTDIR)) {
-                       p11_debug ("config file does not exist");
-                       config = strdup ("\n");
-                       *length = 0;
-                       return_val_if_fail (config != NULL, NULL);
-                       return config;
-
-               } else if ((flags & CONF_IGNORE_ACCESS_DENIED) &&
-                          (error == EPERM || error == EACCES)) {
-                       p11_debug ("config file is inaccessible");
-                       config = strdup ("");
-                       *length = 0;
-                       return_val_if_fail (config != NULL, NULL);
-                       return config;
-               }
-               p11_message ("couldn't open config file: %s: %s", filename,
-                            strerror (error));
-               errno = error;
-               return NULL;
-       }
-
-       /* Figure out size */
-       if (fseek (f, 0, SEEK_END) == -1 ||
-           (len = ftell (f)) == -1 ||
-           fseek (f, 0, SEEK_SET) == -1) {
-               error = errno;
-               p11_message ("couldn't seek config file: %s", filename);
-               errno = error;
-               return NULL;
-       }
-
-       config = malloc (len + 2);
-       if (config == NULL) {
-               p11_message ("config file is too large to read into memory: %lu", len);
-               errno = ENOMEM;
-               return NULL;
-       }
-
-       /* And read in one block */
-       if (fread (config, 1, len, f) != len) {
-               error = errno;
-               p11_message ("couldn't read config file: %s", filename);
-               free (config);
-               errno = error;
-               return NULL;
-       }
-
-       fclose (f);
-
-       /* Null terminate the data */
-       *length = len;
-       return config;
-}
-
 bool
 _p11_conf_merge_defaults (p11_dict *map,
                           p11_dict *defaults)
@@ -165,22 +95,43 @@ p11_dict *
 _p11_conf_parse_file (const char* filename, int flags)
 {
        p11_dict *map = NULL;
-       char *data;
+       void *data;
        p11_lexer lexer;
        bool failed = false;
        size_t length;
+       p11_mmap *mmap;
+       int error;
 
        assert (filename);
 
        p11_debug ("reading config file: %s", filename);
 
-       data = read_config_file (filename, flags, &length);
-       if (!data)
-               return NULL;
+       mmap = p11_mmap_open (filename, &data, &length);
+       if (mmap == NULL) {
+               error = errno;
+               if ((flags & CONF_IGNORE_MISSING) &&
+                   (error == ENOENT || error == ENOTDIR)) {
+                       p11_debug ("config file does not exist");
+
+               } else if ((flags & CONF_IGNORE_ACCESS_DENIED) &&
+                          (error == EPERM || error == EACCES)) {
+                       p11_debug ("config file is inaccessible");
+
+               } else {
+                       p11_message ("couldn't open config file: %s: %s", filename,
+                                    strerror (error));
+                       errno = error;
+                       return NULL;
+               }
+       }
 
        map = p11_dict_new (p11_dict_str_hash, p11_dict_str_equal, free, free);
        return_val_if_fail (map != NULL, NULL);
 
+       /* Empty config fall through above */
+       if (mmap == NULL)
+               return map;
+
        p11_lexer_init (&lexer, filename, data, length);
        while (p11_lexer_next (&lexer, &failed)) {
                switch (lexer.tok_type) {
@@ -210,7 +161,7 @@ _p11_conf_parse_file (const char* filename, int flags)
        }
 
        p11_lexer_done (&lexer);
-       free (data);
+       p11_mmap_close (mmap);
 
        if (failed) {
                p11_dict_free (map);
index b64b737fd72ffd4f212944ba6bf06ac7e4693d51..60571de24bfd8eb1a9f598952b5fe59804e9a74f 100644 (file)
@@ -466,7 +466,7 @@ p11_kit_pin_file_callback (const char *pin_source,
        if (pin_flags & P11_KIT_PIN_FLAGS_RETRY)
                return NULL;
 
-       fd = open (pin_source, O_BINARY | O_RDONLY);
+       fd = open (pin_source, O_BINARY | O_RDONLY | O_CLOEXEC);
        if (fd == -1)
                return NULL;