[gvfs/gnome-3-10] metadata: don't crash if meta_tree_init fails
- From: Ondrej Holy <oholy src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gvfs/gnome-3-10] metadata: don't crash if meta_tree_init fails
- Date: Tue, 6 Jan 2015 10:02:57 +0000 (UTC)
commit 1b0b17c678c452a08666eb110927170b179ad007
Author: Ondrej Holy <oholy redhat com>
Date: Fri Nov 14 10:08:03 2014 +0100
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
client/gdaemonfile.c | 22 ++++++--
client/gdaemonvfs.c | 135 +++++++++++++++++++++++++---------------------
metadata/meta-get-tree.c | 6 ++-
metadata/metatree.c | 89 ++++++++++++++++++++++++-------
metadata/metatree.h | 2 +-
5 files changed, 166 insertions(+), 88 deletions(-)
---
diff --git a/client/gdaemonfile.c b/client/gdaemonfile.c
index f5d28c4..601af40 100644
--- a/client/gdaemonfile.c
+++ b/client/gdaemonfile.c
@@ -836,12 +836,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);
}
@@ -2640,6 +2644,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 7c7f9a7..d9f079f 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.
@@ -1303,69 +1304,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 1af626d..23ce9e4 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 932182d..d6f436d 100644
--- a/metadata/metatree.c
+++ b/metadata/metatree.c
@@ -132,7 +132,7 @@ struct _MetaTree {
MetaJournal *journal;
};
-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,
@@ -347,6 +347,7 @@ meta_tree_init (MetaTree *tree)
guint32 *attributes;
gboolean retried;
int i;
+ int errsv;
retried = FALSE;
retry:
@@ -354,6 +355,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;
@@ -372,13 +375,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;
}
@@ -386,6 +406,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;
}
@@ -397,18 +420,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++;
@@ -417,7 +452,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);
@@ -430,9 +468,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);
@@ -444,6 +480,7 @@ meta_tree_open (const char *filename,
gboolean for_write)
{
MetaTree *tree;
+ gboolean res;
g_assert (sizeof (MetaFileHeader) == 32);
g_assert (sizeof (MetaFileDirEnt) == 16);
@@ -455,7 +492,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;
}
@@ -502,8 +545,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);
@@ -583,7 +629,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 */
@@ -591,16 +637,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 =
@@ -611,9 +660,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 {
@@ -2299,7 +2350,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 f24b4e9..d469b98 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);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
