[gvfs] metadata: don't crash if meta_tree_init fails
- From: Ondrej Holy <oholy src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gvfs] metadata: don't crash if meta_tree_init fails
- Date: Tue, 6 Jan 2015 09:41:08 +0000 (UTC)
commit a03635ee68f043b68c1bb4cb95114af3ecb65ec4
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 e97c9a4..53e5f9a 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 9ab806b..1060c44 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 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 7b59c4a..8875ae2 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 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]
