From 08a1487fb3e271b3c508e14b9d8f64d851e8a80a Mon Sep 17 00:00:00 2001 From: Thomas Martitz Date: Wed, 23 Oct 2013 20:46:03 +0200 Subject: [PATCH 1/3] autoclose: Fix crash due to use-after-free. The signal handler for "sci-notify" needs to be disconnected when the document is closed because the handler might be called afterwards but with already freed user data. --- autoclose/src/autoclose.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/autoclose/src/autoclose.c b/autoclose/src/autoclose.c index 125fad584..4af1e4898 100644 --- a/autoclose/src/autoclose.c +++ b/autoclose/src/autoclose.c @@ -802,11 +802,10 @@ static void on_document_activate(GObject *obj, GeanyDocument *doc, gpointer user_data) { AutocloseUserData *data; - ScintillaObject *sci = NULL; - g_return_if_fail(NULL != doc && NULL != doc->editor); - sci = doc->editor->sci; - g_return_if_fail(NULL != sci); + ScintillaObject *sci; + g_return_if_fail(DOC_VALID(doc)); + sci = doc->editor->sci; data = g_new0(AutocloseUserData, 1); data->doc = doc; plugin_signal_connect(geany_plugin, G_OBJECT(sci), "sci-notify", @@ -820,9 +819,11 @@ on_document_activate(GObject *obj, GeanyDocument *doc, gpointer user_data) static void on_document_close(GObject *obj, GeanyDocument *doc, gpointer user_data) { - /* free the AutocloseUserData instance */ + /* free the AutocloseUserData instance and disconnect the handler */ ScintillaObject *sci = doc->editor->sci; AutocloseUserData *data = g_object_steal_data(G_OBJECT(sci), AC_GOBJECT_KEY); + /* no plugin_signal_disconnect() ?? */ + g_signal_handlers_disconnect_by_func(G_OBJECT(sci), G_CALLBACK(on_editor_notify), data); g_free(data); } From a6f9ce24c4c91d8200321f17871fc4298a0c65d5 Mon Sep 17 00:00:00 2001 From: Thomas Martitz Date: Wed, 23 Oct 2013 20:49:01 +0200 Subject: [PATCH 2/3] autoclose: Rename signal handlers to better fit the signals they're connected to. --- autoclose/src/autoclose.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/autoclose/src/autoclose.c b/autoclose/src/autoclose.c index 4af1e4898..99df118c0 100644 --- a/autoclose/src/autoclose.c +++ b/autoclose/src/autoclose.c @@ -768,7 +768,7 @@ on_key_press(GtkWidget *widget, GdkEventKey *event, gpointer user_data) } static void -on_editor_notify(GObject *obj, gint scn, SCNotification *nt, gpointer user_data) +on_sci_notify(GObject *obj, gint scn, SCNotification *nt, gpointer user_data) { AutocloseUserData *data = user_data; @@ -799,7 +799,7 @@ on_editor_notify(GObject *obj, gint scn, SCNotification *nt, gpointer user_data) #define AC_GOBJECT_KEY "autoclose-userdata" static void -on_document_activate(GObject *obj, GeanyDocument *doc, gpointer user_data) +on_document_open(GObject *obj, GeanyDocument *doc, gpointer user_data) { AutocloseUserData *data; ScintillaObject *sci; @@ -809,7 +809,7 @@ on_document_activate(GObject *obj, GeanyDocument *doc, gpointer user_data) data = g_new0(AutocloseUserData, 1); data->doc = doc; plugin_signal_connect(geany_plugin, G_OBJECT(sci), "sci-notify", - FALSE, G_CALLBACK(on_editor_notify), data); + FALSE, G_CALLBACK(on_sci_notify), data); plugin_signal_connect(geany_plugin, G_OBJECT(sci), "key-press-event", FALSE, G_CALLBACK(on_key_press), data); /* save data pointer via GObject too for on_document_close() */ @@ -823,14 +823,14 @@ on_document_close(GObject *obj, GeanyDocument *doc, gpointer user_data) ScintillaObject *sci = doc->editor->sci; AutocloseUserData *data = g_object_steal_data(G_OBJECT(sci), AC_GOBJECT_KEY); /* no plugin_signal_disconnect() ?? */ - g_signal_handlers_disconnect_by_func(G_OBJECT(sci), G_CALLBACK(on_editor_notify), data); + g_signal_handlers_disconnect_by_func(G_OBJECT(sci), G_CALLBACK(on_sci_notify), data); g_free(data); } PluginCallback plugin_callbacks[] = { - { "document-open", (GCallback) &on_document_activate, FALSE, NULL }, - { "document-new", (GCallback) &on_document_activate, FALSE, NULL }, + { "document-open", (GCallback) &on_document_open, FALSE, NULL }, + { "document-new", (GCallback) &on_document_open, FALSE, NULL }, { "document-close", (GCallback) &on_document_close, FALSE, NULL }, { NULL, NULL, FALSE, NULL } }; @@ -899,7 +899,7 @@ plugin_init(G_GNUC_UNUSED GeanyData *data) int i; foreach_document(i) { - on_document_activate(NULL, documents[i], NULL); + on_document_open(NULL, documents[i], NULL); } GKeyFile *config = g_key_file_new(); From d9e73e61feee780c50d197aade4210419ef3bb7e Mon Sep 17 00:00:00 2001 From: Pavel Roschin Date: Wed, 23 Oct 2013 23:01:13 +0400 Subject: [PATCH 3/3] Homogenize checks, fix doc checking, remove define warnings --- autoclose/src/autoclose.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/autoclose/src/autoclose.c b/autoclose/src/autoclose.c index 99df118c0..8944204ea 100644 --- a/autoclose/src/autoclose.c +++ b/autoclose/src/autoclose.c @@ -52,6 +52,10 @@ PLUGIN_SET_TRANSLATABLE_INFO( "0.2", "Pavel Roschin ") +/* avoid aggresive warnings */ +#undef DOC_VALID +#define DOC_VALID(doc_ptr) (((doc_ptr) && (doc_ptr)->is_valid)) + typedef struct { /* close chars */ gboolean parenthesis; @@ -591,7 +595,7 @@ check_struct( line = sci_get_line_from_position(sci, pos); len = strlen(str); const gchar *sci_buf = get_char_range(sci, get_indent(sci, line), len); - g_return_val_if_fail(NULL != sci_buf, FALSE); + g_return_val_if_fail(sci_buf, FALSE); if (strncmp(sci_buf, str, len) == 0) return TRUE; return FALSE; @@ -623,7 +627,7 @@ check_define( gint line) { const gchar* sci_buf = get_char_range(sci, get_indent(sci, line), 7); - g_return_val_if_fail(NULL != sci_buf, FALSE); + g_return_val_if_fail(sci_buf, FALSE); if (strncmp(sci_buf, "#define", 7) == 0) return TRUE; return FALSE; @@ -645,13 +649,13 @@ auto_close_chars( gboolean has_sel; gint filetype = 0; - g_return_val_if_fail(NULL != data, AC_CONTINUE_ACTION); + g_return_val_if_fail(data, AC_CONTINUE_ACTION); doc = data->doc; - g_return_val_if_fail(NULL != doc, AC_CONTINUE_ACTION); + g_return_val_if_fail(DOC_VALID(doc), AC_CONTINUE_ACTION); editor = doc->editor; - g_return_val_if_fail(NULL != editor, AC_CONTINUE_ACTION); + g_return_val_if_fail(editor, AC_CONTINUE_ACTION); sci = editor->sci; - g_return_val_if_fail(NULL != sci, AC_CONTINUE_ACTION); + g_return_val_if_fail(sci, AC_CONTINUE_ACTION); if (doc->file_type) filetype = doc->file_type->id; @@ -763,7 +767,7 @@ static gboolean on_key_press(GtkWidget *widget, GdkEventKey *event, gpointer user_data) { AutocloseUserData *data = user_data; - g_return_val_if_fail(NULL != data && NULL != data->doc, AC_CONTINUE_ACTION); + g_return_val_if_fail(data && DOC_VALID(data->doc), AC_CONTINUE_ACTION); return auto_close_chars(data, event); } @@ -774,8 +778,8 @@ on_sci_notify(GObject *obj, gint scn, SCNotification *nt, gpointer user_data) if (!ac_info->jump_on_tab) return; - if (!data || !data->doc || !data->doc->editor || !data->doc->editor->sci) - return; + g_return_if_fail(data); + g_return_if_fail(DOC_VALID(data->doc)); ScintillaObject *sci = data->doc->editor->sci; /* reset jump_on_tab state when user clicked away */