[evolution-data-server/gnome-3-18] [IMAPx] Crash in imapx_free_capability()
- From: Milan Crha <mcrha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [evolution-data-server/gnome-3-18] [IMAPx] Crash in imapx_free_capability()
- Date: Mon, 1 Feb 2016 14:18:08 +0000 (UTC)
commit 2b0330f8f386c20632e6edb3760711938b8e76f3
Author: Milan Crha <mcrha redhat com>
Date: Mon Feb 1 15:17:44 2016 +0100
[IMAPx] Crash in imapx_free_capability()
It seems the imapx_disconnect() can be called from multiple threads,
eventually causing use-after-free on is->priv->cinfo. Adding a lock
around the private cinfo structure should avoid the crash.
Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1293028
camel/providers/imapx/camel-imapx-command.c | 2 +-
camel/providers/imapx/camel-imapx-server.c | 154 ++++++++++++++++++++++++---
camel/providers/imapx/camel-imapx-server.h | 6 +
camel/providers/imapx/camel-imapx-store.c | 4 +-
4 files changed, 146 insertions(+), 20 deletions(-)
---
diff --git a/camel/providers/imapx/camel-imapx-command.c b/camel/providers/imapx/camel-imapx-command.c
index 0677736..8862ca7 100644
--- a/camel/providers/imapx/camel-imapx-command.c
+++ b/camel/providers/imapx/camel-imapx-command.c
@@ -420,7 +420,7 @@ camel_imapx_command_add_part (CamelIMAPXCommand *ic,
if (type & CAMEL_IMAPX_COMMAND_LITERAL_PLUS) {
g_string_append_c (buffer, '{');
g_string_append_printf (buffer, "%u", ob_size);
- if (CAMEL_IMAPX_HAVE_CAPABILITY (camel_imapx_server_get_capability_info (ic->is),
LITERALPLUS)) {
+ if (camel_imapx_server_have_capability (ic->is, IMAPX_CAPABILITY_LITERALPLUS)) {
g_string_append_c (buffer, '+');
} else {
type &= ~CAMEL_IMAPX_COMMAND_LITERAL_PLUS;
diff --git a/camel/providers/imapx/camel-imapx-server.c b/camel/providers/imapx/camel-imapx-server.c
index 241dd5a..eb6e85e 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -291,7 +291,7 @@ struct _CamelIMAPXServerPrivate {
gboolean is_cyrus;
- /* Info about the current connection */
+ /* Info about the current connection; guarded by priv->stream_lock */
struct _capability_info *cinfo;
GRecMutex command_lock;
@@ -782,21 +782,36 @@ imapx_untagged_capability (CamelIMAPXServer *is,
GCancellable *cancellable,
GError **error)
{
+ struct _capability_info *cinfo;
+
g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), FALSE);
- if (is->priv->cinfo != NULL)
+ g_mutex_lock (&is->priv->stream_lock);
+
+ if (is->priv->cinfo != NULL) {
imapx_free_capability (is->priv->cinfo);
+ is->priv->cinfo = NULL;
+ }
- is->priv->cinfo = imapx_parse_capability (
- CAMEL_IMAPX_INPUT_STREAM (input_stream), cancellable, error);
+ g_mutex_unlock (&is->priv->stream_lock);
- if (is->priv->cinfo == NULL)
+ cinfo = imapx_parse_capability (CAMEL_IMAPX_INPUT_STREAM (input_stream), cancellable, error);
+
+ if (!cinfo)
return FALSE;
+ g_mutex_lock (&is->priv->stream_lock);
+
+ if (is->priv->cinfo != NULL)
+ imapx_free_capability (is->priv->cinfo);
+ is->priv->cinfo = cinfo;
+
c (is->priv->tagprefix, "got capability flags %08x\n", is->priv->cinfo->capa);
imapx_server_stash_command_arguments (is);
+ g_mutex_unlock (&is->priv->stream_lock);
+
return TRUE;
}
@@ -1789,7 +1804,11 @@ imapx_untagged_ok_no_bad (CamelIMAPXServer *is,
break;
case IMAPX_CAPABILITY:
if (is->priv->context->sinfo->u.cinfo) {
- struct _capability_info *cinfo = is->priv->cinfo;
+ struct _capability_info *cinfo;
+
+ g_mutex_lock (&is->priv->stream_lock);
+
+ cinfo = is->priv->cinfo;
is->priv->cinfo = is->priv->context->sinfo->u.cinfo;
is->priv->context->sinfo->u.cinfo = NULL;
if (cinfo)
@@ -1806,7 +1825,10 @@ imapx_untagged_ok_no_bad (CamelIMAPXServer *is,
is->priv->cinfo->capa &= ~list_extended;
}
}
+
imapx_server_stash_command_arguments (is);
+
+ g_mutex_unlock (&is->priv->stream_lock);
}
break;
case IMAPX_COPYUID:
@@ -2709,7 +2731,11 @@ connected:
goto exit;
}
+ g_mutex_lock (&is->priv->stream_lock);
+
if (!is->priv->cinfo) {
+ g_mutex_unlock (&is->priv->stream_lock);
+
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_CAPABILITY, "CAPABILITY");
success = camel_imapx_server_process_command_sync (is, ic, _("Failed to get capabilities"),
cancellable, error);
@@ -2718,17 +2744,24 @@ connected:
if (!success)
goto exit;
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
if (method == CAMEL_NETWORK_SECURITY_METHOD_STARTTLS_ON_STANDARD_PORT) {
+ g_mutex_lock (&is->priv->stream_lock);
+
if (CAMEL_IMAPX_LACK_CAPABILITY (is->priv->cinfo, STARTTLS)) {
+ g_mutex_unlock (&is->priv->stream_lock);
g_set_error (
&local_error, CAMEL_ERROR,
CAMEL_ERROR_GENERIC,
_("Failed to connect to IMAP server %s in secure mode: %s"),
host, _("STARTTLS not supported"));
goto exit;
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_STARTTLS, "STARTTLS");
@@ -2736,6 +2769,8 @@ connected:
success = camel_imapx_server_process_command_sync (is, ic, _("Failed to issue STARTTLS"),
cancellable, error);
if (success) {
+ g_mutex_lock (&is->priv->stream_lock);
+
/* See if we got new capabilities
* in the STARTTLS response. */
imapx_free_capability (is->priv->cinfo);
@@ -2746,6 +2781,8 @@ connected:
c (is->priv->tagprefix, "got capability flags %08x\n", is->priv->cinfo ?
is->priv->cinfo->capa : 0xFFFFFFFF);
imapx_server_stash_command_arguments (is);
}
+
+ g_mutex_unlock (&is->priv->stream_lock);
}
camel_imapx_command_unref (ic);
@@ -2784,13 +2821,17 @@ connected:
}
/* Get new capabilities if they weren't already given */
+ g_mutex_lock (&is->priv->stream_lock);
if (is->priv->cinfo == NULL) {
+ g_mutex_unlock (&is->priv->stream_lock);
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_CAPABILITY, "CAPABILITY");
success = camel_imapx_server_process_command_sync (is, ic, _("Failed to get
capabilities"), cancellable, error);
camel_imapx_command_unref (ic);
if (!success)
goto exit;
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
}
@@ -2859,7 +2900,11 @@ camel_imapx_server_authenticate_sync (CamelIMAPXServer *is,
g_object_unref (settings);
if (mechanism != NULL) {
+ g_mutex_lock (&is->priv->stream_lock);
+
if (is->priv->cinfo && !g_hash_table_lookup (is->priv->cinfo->auth_types, mechanism)) {
+ g_mutex_unlock (&is->priv->stream_lock);
+
g_set_error (
error, CAMEL_SERVICE_ERROR,
CAMEL_SERVICE_ERROR_CANT_AUTHENTICATE,
@@ -2867,6 +2912,8 @@ camel_imapx_server_authenticate_sync (CamelIMAPXServer *is,
"authentication"), host, mechanism);
result = CAMEL_AUTHENTICATION_ERROR;
goto exit;
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
sasl = camel_sasl_new ("imap", mechanism, service);
@@ -2953,6 +3000,8 @@ camel_imapx_server_authenticate_sync (CamelIMAPXServer *is,
/* Forget old capabilities after login. */
if (result == CAMEL_AUTHENTICATION_ACCEPTED) {
+ g_mutex_lock (&is->priv->stream_lock);
+
if (is->priv->cinfo) {
imapx_free_capability (is->priv->cinfo);
is->priv->cinfo = NULL;
@@ -2964,6 +3013,8 @@ camel_imapx_server_authenticate_sync (CamelIMAPXServer *is,
c (is->priv->tagprefix, "got capability flags %08x\n", is->priv->cinfo ?
is->priv->cinfo->capa : 0xFFFFFFFF);
imapx_server_stash_command_arguments (is);
}
+
+ g_mutex_unlock (&is->priv->stream_lock);
}
camel_imapx_command_unref (ic);
@@ -3020,9 +3071,12 @@ imapx_reconnect (CamelIMAPXServer *is,
goto exception;
/* After login we re-capa unless the server already told us. */
+ g_mutex_lock (&is->priv->stream_lock);
if (is->priv->cinfo == NULL) {
GError *local_error = NULL;
+ g_mutex_unlock (&is->priv->stream_lock);
+
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_CAPABILITY, "CAPABILITY");
camel_imapx_server_process_command_sync (is, ic, _("Failed to get capabilities"),
cancellable, &local_error);
camel_imapx_command_unref (ic);
@@ -3031,15 +3085,20 @@ imapx_reconnect (CamelIMAPXServer *is,
g_propagate_error (error, local_error);
goto exception;
}
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
is->priv->state = IMAPX_AUTHENTICATED;
preauthed:
/* Fetch namespaces (if supported). */
+ g_mutex_lock (&is->priv->stream_lock);
if (CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, NAMESPACE)) {
GError *local_error = NULL;
+ g_mutex_unlock (&is->priv->stream_lock);
+
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_NAMESPACE, "NAMESPACE");
camel_imapx_server_process_command_sync (is, ic, _("Failed to issue NAMESPACE"), cancellable,
&local_error);
camel_imapx_command_unref (ic);
@@ -3048,12 +3107,16 @@ preauthed:
g_propagate_error (error, local_error);
goto exception;
}
+
+ g_mutex_lock (&is->priv->stream_lock);
}
/* Enable quick mailbox resynchronization (if supported). */
if (use_qresync && CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, QRESYNC)) {
GError *local_error = NULL;
+ g_mutex_unlock (&is->priv->stream_lock);
+
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_ENABLE, "ENABLE CONDSTORE QRESYNC");
camel_imapx_server_process_command_sync (is, ic, _("Failed to enable QResync"), cancellable,
&local_error);
camel_imapx_command_unref (ic);
@@ -3063,6 +3126,8 @@ preauthed:
goto exception;
}
+ g_mutex_lock (&is->priv->stream_lock);
+
is->priv->use_qresync = TRUE;
} else {
is->priv->use_qresync = FALSE;
@@ -3072,6 +3137,8 @@ preauthed:
if (CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, NOTIFY)) {
GError *local_error = NULL;
+ g_mutex_unlock (&is->priv->stream_lock);
+
/* XXX The list of FETCH attributes is negotiable. */
ic = camel_imapx_command_new (is, CAMEL_IMAPX_JOB_NOTIFY, "NOTIFY SET "
"(selected "
@@ -3090,8 +3157,12 @@ preauthed:
g_propagate_error (error, local_error);
goto exception;
}
+
+ g_mutex_lock (&is->priv->stream_lock);
}
+ g_mutex_unlock (&is->priv->stream_lock);
+
is->priv->state = IMAPX_INITIALISED;
success = TRUE;
@@ -3101,11 +3172,6 @@ preauthed:
exception:
imapx_disconnect (is);
- if (is->priv->cinfo) {
- imapx_free_capability (is->priv->cinfo);
- is->priv->cinfo = NULL;
- }
-
exit:
g_free (mechanism);
@@ -3762,6 +3828,11 @@ imapx_disconnect (CamelIMAPXServer *is)
g_clear_object (&is->priv->connection);
g_clear_object (&is->priv->subprocess);
+ if (is->priv->cinfo) {
+ imapx_free_capability (is->priv->cinfo);
+ is->priv->cinfo = NULL;
+ }
+
g_mutex_unlock (&is->priv->stream_lock);
g_mutex_lock (&is->priv->select_lock);
@@ -3769,11 +3840,6 @@ imapx_disconnect (CamelIMAPXServer *is)
g_weak_ref_set (&is->priv->select_pending, NULL);
g_mutex_unlock (&is->priv->select_lock);
- if (is->priv->cinfo) {
- imapx_free_capability (is->priv->cinfo);
- is->priv->cinfo = NULL;
- }
-
is->priv->is_cyrus = FALSE;
is->priv->state = IMAPX_DISCONNECTED;
@@ -3807,10 +3873,16 @@ camel_imapx_server_connect_sync (CamelIMAPXServer *is,
if (!imapx_reconnect (is, cancellable, error))
return FALSE;
+ g_mutex_lock (&is->priv->stream_lock);
+
if (CAMEL_IMAPX_LACK_CAPABILITY (is->priv->cinfo, NAMESPACE)) {
+ g_mutex_unlock (&is->priv->stream_lock);
+
/* This also creates a needed faux NAMESPACE */
if (!camel_imapx_server_list_sync (is, "INBOX", 0, cancellable, error))
return FALSE;
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
return TRUE;
@@ -4112,10 +4184,14 @@ camel_imapx_server_copy_message_sync (CamelIMAPXServer *is,
/* If we're moving messages, prefer "UID MOVE" if supported. */
if (delete_originals) {
+ g_mutex_lock (&is->priv->stream_lock);
+
if (CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, MOVE)) {
delete_originals = FALSE;
use_move_command = TRUE;
}
+
+ g_mutex_unlock (&is->priv->stream_lock);
}
source_infos = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, camel_message_info_unref);
@@ -5745,11 +5821,17 @@ camel_imapx_server_update_quota_info_sync (CamelIMAPXServer *is,
g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), FALSE);
g_return_val_if_fail (CAMEL_IS_IMAPX_MAILBOX (mailbox), FALSE);
+ g_mutex_lock (&is->priv->stream_lock);
+
if (CAMEL_IMAPX_LACK_CAPABILITY (is->priv->cinfo, QUOTA)) {
+ g_mutex_unlock (&is->priv->stream_lock);
+
g_set_error_literal (
error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
_("IMAP server does not support quotas"));
return FALSE;
+ } else {
+ g_mutex_unlock (&is->priv->stream_lock);
}
success = camel_imapx_server_ensure_selected_sync (is, mailbox, cancellable, error);
@@ -6020,9 +6102,14 @@ camel_imapx_server_can_use_idle (CamelIMAPXServer *is)
{
gboolean use_idle = FALSE;
+ g_mutex_lock (&is->priv->stream_lock);
+
/* No need for IDLE if the server supports NOTIFY. */
- if (CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, NOTIFY))
+ if (CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, NOTIFY)) {
+ g_mutex_unlock (&is->priv->stream_lock);
+
return FALSE;
+ }
if (CAMEL_IMAPX_HAVE_CAPABILITY (is->priv->cinfo, IDLE)) {
CamelIMAPXSettings *settings;
@@ -6032,6 +6119,8 @@ camel_imapx_server_can_use_idle (CamelIMAPXServer *is)
g_object_unref (settings);
}
+ g_mutex_unlock (&is->priv->stream_lock);
+
return use_idle;
}
@@ -6282,6 +6371,7 @@ camel_imapx_server_register_untagged_handler (CamelIMAPXServer *is,
return previous;
}
+/* This function is not thread-safe. */
const struct _capability_info *
camel_imapx_server_get_capability_info (CamelIMAPXServer *is)
{
@@ -6290,6 +6380,36 @@ camel_imapx_server_get_capability_info (CamelIMAPXServer *is)
return is->priv->cinfo;
}
+gboolean
+camel_imapx_server_have_capability (CamelIMAPXServer *is,
+ guint32 capability)
+{
+ gboolean have;
+
+ g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), FALSE);
+
+ g_mutex_lock (&is->priv->stream_lock);
+ have = is->priv->cinfo != NULL && (is->priv->cinfo->capa & capability) != 0;
+ g_mutex_unlock (&is->priv->stream_lock);
+
+ return have;
+}
+
+gboolean
+camel_imapx_server_lack_capability (CamelIMAPXServer *is,
+ guint32 capability)
+{
+ gboolean lack;
+
+ g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), FALSE);
+
+ g_mutex_lock (&is->priv->stream_lock);
+ lack = is->priv->cinfo != NULL && (is->priv->cinfo->capa & capability) == 0;
+ g_mutex_unlock (&is->priv->stream_lock);
+
+ return lack;
+}
+
gchar
camel_imapx_server_get_tagprefix (CamelIMAPXServer *is)
{
diff --git a/camel/providers/imapx/camel-imapx-server.h b/camel/providers/imapx/camel-imapx-server.h
index a51c39d..16d5090 100644
--- a/camel/providers/imapx/camel-imapx-server.h
+++ b/camel/providers/imapx/camel-imapx-server.h
@@ -135,6 +135,12 @@ CamelIMAPXMailbox *
const struct _capability_info *
camel_imapx_server_get_capability_info
(CamelIMAPXServer *is);
+gboolean camel_imapx_server_have_capability
+ (CamelIMAPXServer *is,
+ guint32 capability);
+gboolean camel_imapx_server_lack_capability
+ (CamelIMAPXServer *is,
+ guint32 capability);
gchar camel_imapx_server_get_tagprefix
(CamelIMAPXServer *is);
void camel_imapx_server_set_tagprefix
diff --git a/camel/providers/imapx/camel-imapx-store.c b/camel/providers/imapx/camel-imapx-store.c
index d426113..bf99e25 100644
--- a/camel/providers/imapx/camel-imapx-store.c
+++ b/camel/providers/imapx/camel-imapx-store.c
@@ -3223,7 +3223,7 @@ camel_imapx_store_handle_list_response (CamelIMAPXStore *imapx_store,
/* Fabricate a CamelIMAPXNamespaceResponse if the server lacks the
* NAMESPACE capability and this is the first LIST / LSUB response. */
- if (CAMEL_IMAPX_LACK_CAPABILITY (camel_imapx_server_get_capability_info (imapx_server), NAMESPACE)) {
+ if (camel_imapx_server_lack_capability (imapx_server, IMAPX_CAPABILITY_NAMESPACE)) {
g_mutex_lock (&imapx_store->priv->namespaces_lock);
if (imapx_store->priv->namespaces == NULL) {
imapx_store->priv->namespaces = camel_imapx_namespace_response_faux_new (response);
@@ -3287,7 +3287,7 @@ camel_imapx_store_handle_lsub_response (CamelIMAPXStore *imapx_store,
/* Fabricate a CamelIMAPXNamespaceResponse if the server lacks the
* NAMESPACE capability and this is the first LIST / LSUB response. */
- if (CAMEL_IMAPX_LACK_CAPABILITY (camel_imapx_server_get_capability_info (imapx_server), NAMESPACE)) {
+ if (camel_imapx_server_lack_capability (imapx_server, IMAPX_CAPABILITY_NAMESPACE)) {
g_mutex_lock (&imapx_store->priv->namespaces_lock);
if (imapx_store->priv->namespaces == NULL) {
imapx_store->priv->namespaces = camel_imapx_namespace_response_faux_new (response);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
