diff options
author | Ondrej Holy <oholy@redhat.com> | 2014-11-14 10:08:03 +0100 |
---|---|---|
committer | Ondrej Holy <oholy@redhat.com> | 2015-01-06 10:38:40 +0100 |
commit | a03635ee68f043b68c1bb4cb95114af3ecb65ec4 (patch) | |
tree | 037d01373d40a8d30ebc1696133e59a19be816c9 | |
parent | 7ea61cb7e5ff41f4abf228df3d9539ce3c07d18b (diff) | |
download | gvfs-a03635ee68f043b68c1bb4cb95114af3ecb65ec4.tar.gz |
metadata: don't crash if meta_tree_init fails
It can fail when e.g. database file is corrupted or doesn't have
correct permissions. This patch also adds warnings to be possible
determine reason why initialization failed.
It is based on patches from Matthew W. S. Bell and Ross Lagerwall.
https://bugzilla.gnome.org/show_bug.cgi?id=598561
-rw-r--r-- | client/gdaemonfile.c | 22 | ||||
-rw-r--r-- | client/gdaemonvfs.c | 135 | ||||
-rw-r--r-- | metadata/meta-get-tree.c | 6 | ||||
-rw-r--r-- | metadata/metatree.c | 89 | ||||
-rw-r--r-- | metadata/metatree.h | 2 |
5 files changed, 166 insertions, 88 deletions
diff --git a/client/gdaemonfile.c b/client/gdaemonfile.c index e97c9a4e..53e5f9a0 100644 --- a/client/gdaemonfile.c +++ b/client/gdaemonfile.c @@ -830,12 +830,16 @@ add_metadata (GFile *file, tree = meta_tree_lookup_by_name (treename, FALSE); g_free (treename); - g_file_info_set_attribute_mask (info, matcher); - meta_tree_enumerate_keys (tree, daemon_file->path, - enumerate_keys_callback, info); - g_file_info_unset_attribute_mask (info); + if (tree) + { + g_file_info_set_attribute_mask (info, matcher); + meta_tree_enumerate_keys (tree, daemon_file->path, + enumerate_keys_callback, info); + g_file_info_unset_attribute_mask (info); + + meta_tree_unref (tree); + } - meta_tree_unref (tree); g_file_attribute_matcher_unref (matcher); } @@ -2649,6 +2653,14 @@ set_metadata_attribute (GFile *file, tree = meta_tree_lookup_by_name (treename, FALSE); g_free (treename); + if (!tree) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + _("Error setting file metadata: %s"), + _("can't open metadata tree")); + return FALSE; + } + res = FALSE; proxy = _g_daemon_vfs_get_metadata_proxy (cancellable, error); diff --git a/client/gdaemonvfs.c b/client/gdaemonvfs.c index 9ab806b3..1060c440 100644 --- a/client/gdaemonvfs.c +++ b/client/gdaemonvfs.c @@ -1,3 +1,4 @@ + /* GIO - GLib Input, Output and Streaming Library * * Copyright (C) 2006-2007 Red Hat, Inc. @@ -1279,69 +1280,79 @@ g_daemon_vfs_local_file_set_attributes (GVfs *vfs, statbuf.st_dev, FALSE, &tree_path); - - proxy = _g_daemon_vfs_get_metadata_proxy (NULL, error); - if (proxy == NULL) - { - res = FALSE; + if (!tree) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + _("Error setting file metadata: %s"), + _("can't open metadata tree")); + res = FALSE; error = NULL; /* Don't set further errors */ - } - else - { - builder = g_variant_builder_new (G_VARIANT_TYPE_VARDICT); - metatreefile = meta_tree_get_filename (tree); - num_set = 0; - - for (i = 0; attributes[i] != NULL; i++) - { - if (g_file_info_get_attribute_data (info, attributes[i], &type, &value, NULL)) - { - appended = _g_daemon_vfs_append_metadata_for_set (builder, - tree, - tree_path, - attributes[i], - type, - value); - if (appended != -1) - { - num_set += appended; - g_file_info_set_attribute_status (info, attributes[i], - G_FILE_ATTRIBUTE_STATUS_SET); - } - else - { - res = FALSE; - g_set_error (error, G_IO_ERROR, - G_IO_ERROR_INVALID_ARGUMENT, - _("Error setting file metadata: %s"), - _("values must be string or list of strings")); - error = NULL; /* Don't set further errors */ - g_file_info_set_attribute_status (info, attributes[i], - G_FILE_ATTRIBUTE_STATUS_ERROR_SETTING); - } - } - } - - if (num_set > 0 && - ! gvfs_metadata_call_set_sync (proxy, - metatreefile, - tree_path, - g_variant_builder_end (builder), - NULL, - error)) - { - res = FALSE; - error = NULL; /* Don't set further errors */ - for (i = 0; attributes[i] != NULL; i++) - g_file_info_set_attribute_status (info, attributes[i], - G_FILE_ATTRIBUTE_STATUS_ERROR_SETTING); - } - - g_variant_builder_unref (builder); - - meta_lookup_cache_free (cache); - meta_tree_unref (tree); - g_free (tree_path); + } + else + { + proxy = _g_daemon_vfs_get_metadata_proxy (NULL, error); + if (proxy == NULL) + { + res = FALSE; + error = NULL; /* Don't set further errors */ + } + else + { + builder = g_variant_builder_new (G_VARIANT_TYPE_VARDICT); + metatreefile = meta_tree_get_filename (tree); + num_set = 0; + + for (i = 0; attributes[i] != NULL; i++) + { + if (g_file_info_get_attribute_data (info, attributes[i], &type, &value, NULL)) + { + appended = _g_daemon_vfs_append_metadata_for_set (builder, + tree, + tree_path, + attributes[i], + type, + value); + if (appended != -1) + { + num_set += appended; + g_file_info_set_attribute_status (info, attributes[i], + G_FILE_ATTRIBUTE_STATUS_SET); + } + else + { + res = FALSE; + g_set_error (error, G_IO_ERROR, + G_IO_ERROR_INVALID_ARGUMENT, + _("Error setting file metadata: %s"), + _("values must be string or list of strings")); + error = NULL; /* Don't set further errors */ + g_file_info_set_attribute_status (info, attributes[i], + G_FILE_ATTRIBUTE_STATUS_ERROR_SETTING); + } + } + } + + if (num_set > 0 && + ! gvfs_metadata_call_set_sync (proxy, + metatreefile, + tree_path, + g_variant_builder_end (builder), + NULL, + error)) + { + res = FALSE; + error = NULL; /* Don't set further errors */ + for (i = 0; attributes[i] != NULL; i++) + g_file_info_set_attribute_status (info, attributes[i], + G_FILE_ATTRIBUTE_STATUS_ERROR_SETTING); + } + + g_variant_builder_unref (builder); + + meta_lookup_cache_free (cache); + meta_tree_unref (tree); + g_free (tree_path); + } } } diff --git a/metadata/meta-get-tree.c b/metadata/meta-get-tree.c index 1af626da..23ce9e42 100644 --- a/metadata/meta-get-tree.c +++ b/metadata/meta-get-tree.c @@ -46,7 +46,11 @@ main (int argc, tree_path = NULL; tree = meta_lookup_cache_lookup_path (cache, argv[i], statbuf.st_dev, FALSE, &tree_path); - g_print ("tree: %s (exists: %d), tree path: %s\n", meta_tree_get_filename (tree), meta_tree_exists (tree), tree_path); + if (tree) + g_print ("tree: %s (exists: %d), tree path: %s\n", meta_tree_get_filename (tree), meta_tree_exists (tree), tree_path); + else + g_print ("tree lookup failed\n"); + if (pause) { char buffer[1000]; diff --git a/metadata/metatree.c b/metadata/metatree.c index 7b59c4a5..8875ae26 100644 --- a/metadata/metatree.c +++ b/metadata/metatree.c @@ -153,7 +153,7 @@ static inline guint64 ldq_u(guint64 *p) #define ldq_u(x) (*(x)) #endif -static void meta_tree_refresh_locked (MetaTree *tree, +static gboolean meta_tree_refresh_locked (MetaTree *tree, gboolean force_reread); static MetaJournal *meta_journal_open (MetaTree *tree, const char *filename, @@ -368,6 +368,7 @@ meta_tree_init (MetaTree *tree) guint32 *attributes; gboolean retried; int i; + int errsv; retried = FALSE; retry: @@ -375,6 +376,8 @@ meta_tree_init (MetaTree *tree) fd = safe_open (tree, tree->filename, O_RDONLY); if (fd == -1) { + errsv = errno; + if (tree->for_write && !retried) { MetaBuilder *builder; @@ -393,13 +396,30 @@ meta_tree_init (MetaTree *tree) } meta_builder_free (builder); } + else if (tree->for_write || errsv != ENOENT) + { + g_warning ("can't init metadata tree %s: open: %s", tree->filename, g_strerror (errsv)); + } tree->fd = -1; + + /* If we're opening for reading and the file does not exist, it is not + * an error. The file will be created later. */ + return !tree->for_write && errsv == ENOENT; + } + + if (fstat (fd, &statbuf) != 0) + { + errsv = errno; + g_warning ("can't init metadata tree %s: fstat: %s", tree->filename, g_strerror (errsv)); + + close (fd); return FALSE; } - if (fstat (fd, &statbuf) != 0 || - statbuf.st_size < sizeof (MetaFileHeader)) + if (statbuf.st_size < sizeof (MetaFileHeader)) { + g_warning ("can't init metadata tree %s: wrong size", tree->filename); + close (fd); return FALSE; } @@ -407,6 +427,9 @@ meta_tree_init (MetaTree *tree) data = mmap (NULL, statbuf.st_size, PROT_READ, MAP_SHARED, fd, 0); if (data == MAP_FAILED) { + errsv = errno; + g_warning ("can't init metadata tree %s: mmap: %s", tree->filename, g_strerror (errsv)); + close (fd); return FALSE; } @@ -418,18 +441,30 @@ meta_tree_init (MetaTree *tree) tree->header = (MetaFileHeader *)data; if (memcmp (tree->header->magic, MAGIC, MAGIC_LEN) != 0) - goto err; + { + g_warning ("can't init metadata tree %s: wrong magic", tree->filename); + goto err; + } if (tree->header->major != MAJOR_VERSION) - goto err; + { + g_warning ("can't init metadata tree %s: wrong version", tree->filename); + goto err; + } tree->root = verify_block_pointer (tree, tree->header->root, sizeof (MetaFileDirEnt)); if (tree->root == NULL) - goto err; + { + g_warning ("can't init metadata tree %s: wrong pointer", tree->filename); + goto err; + } attributes = verify_array_block (tree, tree->header->attributes, sizeof (guint32)); if (attributes == NULL) - goto err; + { + g_warning ("can't init metadata tree %s: wrong block", tree->filename); + goto err; + } tree->num_attributes = GUINT32_FROM_BE (*attributes); attributes++; @@ -438,7 +473,10 @@ meta_tree_init (MetaTree *tree) { tree->attributes[i] = verify_string (tree, attributes[i]); if (tree->attributes[i] == NULL) - goto err; + { + g_warning ("can't init metadata tree %s: wrong attribute", tree->filename); + goto err; + } } tree->tag = GUINT32_FROM_BE (tree->header->random_tag); @@ -451,9 +489,7 @@ meta_tree_init (MetaTree *tree) journal. However we can detect this case by looking at the tree and see if its been rotated, we do this to ensure we have an uptodate tree+journal combo. */ - meta_tree_refresh_locked (tree, FALSE); - - return TRUE; + return meta_tree_refresh_locked (tree, FALSE); err: meta_tree_clear (tree); @@ -465,6 +501,7 @@ meta_tree_open (const char *filename, gboolean for_write) { MetaTree *tree; + gboolean res; g_assert (sizeof (MetaFileHeader) == 32); g_assert (sizeof (MetaFileDirEnt) == 16); @@ -476,7 +513,13 @@ meta_tree_open (const char *filename, tree->for_write = for_write; tree->fd = -1; - meta_tree_init (tree); + res = meta_tree_init (tree); + if (!res) + { + /* do not return uninitialized tree to avoid corruptions */ + meta_tree_unref (tree); + tree = NULL; + } return tree; } @@ -523,8 +566,11 @@ meta_tree_lookup_by_name (const char *name, meta_tree_ref (tree); G_UNLOCK (cached_trees); - meta_tree_refresh (tree); - return tree; + if (meta_tree_refresh (tree)) + return tree; + + meta_tree_unref (tree); + tree = NULL; } filename = g_build_filename (g_get_user_data_dir (), "gvfs-metadata", name, NULL); @@ -604,7 +650,7 @@ meta_tree_has_new_journal_entries (MetaTree *tree) /* Must be called with a write lock held */ -static void +static gboolean meta_tree_refresh_locked (MetaTree *tree, gboolean force_reread) { /* Needs to recheck since we dropped read lock */ @@ -612,16 +658,19 @@ meta_tree_refresh_locked (MetaTree *tree, gboolean force_reread) { if (tree->header) meta_tree_clear (tree); - meta_tree_init (tree); + return meta_tree_init (tree); } else if (meta_tree_has_new_journal_entries (tree)) meta_journal_validate_more (tree->journal); + + return TRUE; } -void +gboolean meta_tree_refresh (MetaTree *tree) { gboolean needs_refresh; + gboolean res = TRUE; g_rw_lock_reader_lock (&metatree_lock); needs_refresh = @@ -632,9 +681,11 @@ meta_tree_refresh (MetaTree *tree) if (needs_refresh) { g_rw_lock_writer_lock (&metatree_lock); - meta_tree_refresh_locked (tree, FALSE); + res = meta_tree_refresh_locked (tree, FALSE); g_rw_lock_writer_unlock (&metatree_lock); } + + return res; } struct FindName { @@ -2320,7 +2371,7 @@ meta_tree_flush_locked (MetaTree *tree) meta_tree_get_filename (tree)); if (res) /* Force re-read since we wrote a new file */ - meta_tree_refresh_locked (tree, TRUE); + res = meta_tree_refresh_locked (tree, TRUE); meta_builder_free (builder); diff --git a/metadata/metatree.h b/metadata/metatree.h index f24b4e9b..d469b98e 100644 --- a/metadata/metatree.h +++ b/metadata/metatree.h @@ -63,7 +63,7 @@ MetaTree * meta_tree_lookup_by_name (const char *name, gboolean for_write); MetaTree * meta_tree_ref (MetaTree *tree); void meta_tree_unref (MetaTree *tree); -void meta_tree_refresh (MetaTree *tree); +gboolean meta_tree_refresh (MetaTree *tree); const char *meta_tree_get_filename (MetaTree *tree); gboolean meta_tree_exists (MetaTree *tree); gboolean meta_tree_is_on_nfs (MetaTree *tree); |