From: Timo Sirainen Date: Tue, 8 Jun 2010 18:34:14 +0000 (+0100) Subject: auth: Changed how auth deinitilization works. X-Git-Tag: 2.0.beta6~32 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9d75363d3fbabc2fbc2d80f06672e3ed8965804a;p=thirdparty%2Fdovecot%2Fcore.git auth: Changed how auth deinitilization works. --HG-- branch : HEAD --- diff --git a/src/auth/auth-client-connection.c b/src/auth/auth-client-connection.c index 4c70aedafe..cf83820699 100644 --- a/src/auth/auth-client-connection.c +++ b/src/auth/auth-client-connection.c @@ -319,7 +319,9 @@ auth_client_connection_create(struct auth *auth, int fd, bool login_requests) return conn; } -void auth_client_connection_destroy(struct auth_client_connection **_conn) +static void +auth_client_connection_destroy_full(struct auth_client_connection **_conn, + bool abort_requests) { struct auth_client_connection *conn = *_conn; struct auth_client_connection *const *clients; @@ -347,13 +349,21 @@ void auth_client_connection_destroy(struct auth_client_connection **_conn) net_disconnect(conn->fd); conn->fd = -1; - if (conn->request_handler != NULL) - auth_request_handler_destroy(&conn->request_handler); + if (conn->request_handler != NULL) { + if (abort_requests) + auth_request_handler_abort_requests(conn->request_handler); + auth_request_handler_unref(&conn->request_handler); + } master_service_client_connection_destroyed(master_service); auth_client_connection_unref(&conn); } +void auth_client_connection_destroy(struct auth_client_connection **_conn) +{ + auth_client_connection_destroy_full(_conn, TRUE); +} + static void auth_client_connection_unref(struct auth_client_connection **_conn) { struct auth_client_connection *conn = *_conn; @@ -394,6 +404,6 @@ void auth_client_connections_deinit(void) clients = array_get_modifiable(&auth_client_connections, &count); for (i = count; i > 0; i--) - auth_client_connection_destroy(&clients[i-1]); + auth_client_connection_destroy_full(&clients[i-1], FALSE); array_free(&auth_client_connections); } diff --git a/src/auth/auth-master-connection.c b/src/auth/auth-master-connection.c index bf16b69346..696142a363 100644 --- a/src/auth/auth-master-connection.c +++ b/src/auth/auth-master-connection.c @@ -3,7 +3,6 @@ #include "auth-common.h" #include "array.h" #include "hash.h" -#include "llist.h" #include "str.h" #include "strescape.h" #include "str-sanitize.h" @@ -27,12 +26,6 @@ #define MAX_INBUF_SIZE 1024 #define MAX_OUTBUF_SIZE (1024*50) -struct auth_request_list { - struct auth_request_list *prev, *next; - - struct auth_request *auth_request; -}; - struct master_userdb_request { struct auth_master_connection *conn; unsigned int id; @@ -123,7 +116,6 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args, const char **error_r) { struct auth_request *auth_request; - struct auth_request_list *request_list; const char *const *list, *name, *arg; unsigned int id; @@ -140,11 +132,6 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args, auth_request->master = conn; auth_master_connection_ref(conn); - request_list = p_new(auth_request->pool, struct auth_request_list, 1); - request_list->auth_request = auth_request; - DLLIST_PREPEND(&conn->requests, request_list); - auth_request->context = request_list; - if (!auth_request_set_username(auth_request, list[1], error_r)) { *request_r = auth_request; return 0; @@ -165,7 +152,6 @@ master_input_auth_request(struct auth_master_connection *conn, const char *args, if (auth_request->service == NULL) { i_error("BUG: Master sent %s request without service", cmd); - DLLIST_REMOVE(&conn->requests, request_list); auth_request_unref(&auth_request); auth_master_connection_unref(&conn); return -1; @@ -181,7 +167,6 @@ user_callback(enum userdb_result result, struct auth_request *auth_request) { struct auth_master_connection *conn = auth_request->master; - struct auth_request_list *list = auth_request->context; struct auth_stream_reply *reply = auth_request->userdb_reply; string_t *str; const char *value; @@ -214,7 +199,6 @@ user_callback(enum userdb_result result, str_append_c(str, '\n'); (void)o_stream_send(conn->output, str_data(str), str_len(str)); - DLLIST_REMOVE(&conn->requests, list); auth_request_unref(&auth_request); auth_master_connection_unref(&conn); } @@ -247,7 +231,6 @@ pass_callback(enum passdb_result result, struct auth_request *auth_request) { struct auth_master_connection *conn = auth_request->master; - struct auth_request_list *list = auth_request->context; struct auth_stream_reply *reply = auth_request->extra_fields; string_t *str; @@ -276,7 +259,6 @@ pass_callback(enum passdb_result result, str_append_c(str, '\n'); (void)o_stream_send(conn->output, str_data(str), str_len(str)); - DLLIST_REMOVE(&conn->requests, list); auth_request_unref(&auth_request); auth_master_connection_unref(&conn); } @@ -532,7 +514,6 @@ void auth_master_connection_destroy(struct auth_master_connection **_conn) { struct auth_master_connection *conn = *_conn; struct auth_master_connection *const *masters; - struct auth_request_list *list; unsigned int idx; *_conn = NULL; @@ -540,9 +521,6 @@ void auth_master_connection_destroy(struct auth_master_connection **_conn) return; conn->destroyed = TRUE; - for (list = conn->requests; list != NULL; list = list->next) - list->auth_request->destroyed = TRUE; - array_foreach(&auth_master_connections, masters) { if (*masters == conn) { idx = array_foreach_idx(&auth_master_connections, diff --git a/src/auth/auth-request-handler.c b/src/auth/auth-request-handler.c index 8cf62d115a..4f57f72556 100644 --- a/src/auth/auth-request-handler.c +++ b/src/auth/auth-request-handler.c @@ -29,8 +29,6 @@ struct auth_request_handler { void *context; auth_request_callback_t *master_callback; - - unsigned int destroyed:1; }; static ARRAY_DEFINE(auth_failures_arr, struct auth_request *); @@ -59,60 +57,38 @@ auth_request_handler_create(auth_request_callback_t *callback, void *context, return handler; } -static void auth_request_handler_unref(struct auth_request_handler **_handler) +void auth_request_handler_abort_requests(struct auth_request_handler *handler) { - struct auth_request_handler *handler = *_handler; struct hash_iterate_context *iter; void *key, *value; - *_handler = NULL; - i_assert(handler->refcount > 0); - if (--handler->refcount > 0) - return; - iter = hash_table_iterate_init(handler->requests); while (hash_table_iterate(iter, &key, &value)) { struct auth_request *auth_request = value; - auth_request->destroyed = TRUE; auth_request_unref(&auth_request); } hash_table_iterate_deinit(&iter); hash_table_clear(handler->requests, TRUE); - - /* notify parent that we're done with all requests */ - handler->callback(NULL, handler->context); - - hash_table_destroy(&handler->requests); - pool_unref(&handler->pool); } -static void -auth_request_handler_destroy_requests(struct auth_request_handler *handler) -{ - struct hash_iterate_context *iter; - void *key, *value; - - iter = hash_table_iterate_init(handler->requests); - while (hash_table_iterate(iter, &key, &value)) { - struct auth_request *auth_request = value; - - auth_request->destroyed = TRUE; - } - hash_table_iterate_deinit(&iter); -} - -void auth_request_handler_destroy(struct auth_request_handler **_handler) +void auth_request_handler_unref(struct auth_request_handler **_handler) { struct auth_request_handler *handler = *_handler; *_handler = NULL; - i_assert(!handler->destroyed); + i_assert(handler->refcount > 0); + if (--handler->refcount > 0) + return; + + i_assert(hash_table_count(handler->requests) == 0); + + /* notify parent that we're done with all requests */ + handler->callback(NULL, handler->context); - handler->destroyed = TRUE; - auth_request_handler_destroy_requests(handler); - auth_request_handler_unref(&handler); + hash_table_destroy(&handler->requests); + pool_unref(&handler->pool); } void auth_request_handler_set(struct auth_request_handler *handler, @@ -198,8 +174,7 @@ auth_request_handle_failure(struct auth_request *request, auth_request_handler_remove(handler, request); if (request->no_failure_delay) { - /* passdb specifically requested not to delay the - reply. */ + /* passdb specifically requested not to delay the reply. */ handler->callback(reply, handler->context); auth_request_unref(&request); return; diff --git a/src/auth/auth-request-handler.h b/src/auth/auth-request-handler.h index 834c311fad..e9bc847fa6 100644 --- a/src/auth/auth-request-handler.h +++ b/src/auth/auth-request-handler.h @@ -29,7 +29,8 @@ auth_request_handler_create(auth_request_callback_t *callback, void *context, (auth_request_callback_t *)callback, context, \ master_callback) #endif -void auth_request_handler_destroy(struct auth_request_handler **handler); +void auth_request_handler_unref(struct auth_request_handler **handler); +void auth_request_handler_abort_requests(struct auth_request_handler *handler); void auth_request_handler_set(struct auth_request_handler *handler, unsigned int connect_uid, diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c index 7c922da0ca..b8fdebf858 100644 --- a/src/auth/auth-request.c +++ b/src/auth/auth-request.c @@ -389,13 +389,6 @@ auth_request_handle_passdb_callback(enum passdb_result *result, strlen(request->passdb_password)); } - if (request->destroyed) { - /* the passdb may have been freed already. this request won't - be sent anywhere anyway, so just fail it immediately. */ - *result = PASSDB_RESULT_INTERNAL_FAILURE; - return TRUE; - } - if (request->passdb->set->deny && *result != PASSDB_RESULT_USER_UNKNOWN) { /* deny passdb. we can get through this step only if the @@ -559,9 +552,14 @@ void auth_request_verify_plain(struct auth_request *request, auth_request_set_state(request, AUTH_REQUEST_STATE_PASSDB); request->credentials_scheme = NULL; - if (passdb->blocking) + if (passdb->iface.verify_plain == NULL) { + /* we're deinitializing and just want to get rid of this + request */ + auth_request_verify_plain_callback( + PASSDB_RESULT_INTERNAL_FAILURE, request); + } else if (passdb->blocking) { passdb_blocking_verify_plain(request); - else { + } else if (passdb->iface.verify_plain != NULL) { passdb->iface.verify_plain(request, password, auth_request_verify_plain_callback); } @@ -741,14 +739,6 @@ void auth_request_userdb_callback(enum userdb_result result, { struct userdb_module *userdb = request->userdb->userdb; - if (request->destroyed) { - /* the userdb may have been freed already. this request won't - be sent anywhere anyway, so just fail it immediately. */ - request->private_callback. - userdb(USERDB_RESULT_INTERNAL_FAILURE, request); - return; - } - if (result != USERDB_RESULT_OK && request->userdb->next != NULL) { /* try next userdb. */ if (result == USERDB_RESULT_INTERNAL_FAILURE) @@ -823,7 +813,11 @@ void auth_request_lookup_user(struct auth_request *request, } } - if (userdb->blocking) + if (userdb->iface->lookup == NULL) { + /* we are deinitializing */ + auth_request_userdb_callback(USERDB_RESULT_INTERNAL_FAILURE, + request); + } else if (userdb->blocking) userdb_blocking_lookup(request); else userdb->iface->lookup(request, auth_request_userdb_callback); diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h index a08708e11a..3e7bc47285 100644 --- a/src/auth/auth-request.h +++ b/src/auth/auth-request.h @@ -109,7 +109,6 @@ struct auth_request { unsigned int userdb_lookup:1; unsigned int userdb_lookup_failed:1; unsigned int secured:1; - unsigned int destroyed:1; /* ... mechanism specific data ... */ }; diff --git a/src/auth/auth.c b/src/auth/auth.c index e8175cc458..3b6cb039c2 100644 --- a/src/auth/auth.c +++ b/src/auth/auth.c @@ -47,7 +47,7 @@ auth_userdb_preinit(struct auth *auth, const struct auth_userdb_settings *set) userdb_preinit(auth->pool, set->driver, set->args); } -struct auth * +static struct auth * auth_preinit(const struct auth_settings *set, const char *service, pool_t pool, const struct mechanisms_register *reg) { @@ -185,7 +185,7 @@ static void auth_mech_list_verify_passdb(struct auth *auth) } } -void auth_init(struct auth *auth) +static void auth_init(struct auth *auth) { struct auth_passdb *passdb; struct auth_userdb *userdb; @@ -200,22 +200,17 @@ void auth_init(struct auth *auth) auth_mech_list_verify_passdb(auth); } -void auth_deinit(struct auth **_auth) +static void auth_deinit(struct auth *auth) { - struct auth *auth = *_auth; struct auth_passdb *passdb; struct auth_userdb *userdb; - *_auth = NULL; - for (passdb = auth->masterdbs; passdb != NULL; passdb = passdb->next) passdb_deinit(passdb->passdb); for (passdb = auth->passdbs; passdb != NULL; passdb = passdb->next) passdb_deinit(passdb->passdb); for (userdb = auth->userdbs; userdb != NULL; userdb = userdb->next) userdb_deinit(userdb->userdb); - - pool_unref(&auth->pool); } struct auth *auth_find_service(const char *name) @@ -279,6 +274,14 @@ void auths_init(void) } void auths_deinit(void) +{ + struct auth *const *auth; + + array_foreach(&auths, auth) + auth_deinit(*auth); +} + +void auths_free(void) { struct auth **auth; unsigned int i, count; @@ -287,6 +290,6 @@ void auths_deinit(void) the first auth pool that used them */ auth = array_get_modifiable(&auths, &count); for (i = count; i > 0; i--) - auth_deinit(&auth[i-1]); + pool_unref(&auth[i-1]->pool); array_free(&auths); } diff --git a/src/auth/auth.h b/src/auth/auth.h index 2f8ab37b0e..4c96c082d7 100644 --- a/src/auth/auth.h +++ b/src/auth/auth.h @@ -32,12 +32,6 @@ struct auth { extern struct auth_penalty *auth_penalty; -struct auth * -auth_preinit(const struct auth_settings *set, const char *service, pool_t pool, - const struct mechanisms_register *mech_reg); -void auth_init(struct auth *auth); -void auth_deinit(struct auth **auth); - struct auth *auth_find_service(const char *name); void auths_preinit(const struct auth_settings *set, pool_t pool, @@ -45,5 +39,6 @@ void auths_preinit(const struct auth_settings *set, pool_t pool, const char *const *services); void auths_init(void); void auths_deinit(void); +void auths_free(void); #endif diff --git a/src/auth/main.c b/src/auth/main.c index eed2bb6d2a..ad538cc2c7 100644 --- a/src/auth/main.c +++ b/src/auth/main.c @@ -151,23 +151,20 @@ static void main_init(void) static void main_deinit(void) { - if (auth_worker_client != NULL) - auth_worker_client_destroy(&auth_worker_client); - else - auth_request_handler_flush_failures(TRUE); + /* deinit auth workers, which aborts pending requests */ + auth_worker_server_deinit(); + /* deinit passdbs and userdbs. it aborts any pending async requests. */ + auths_deinit(); + /* flush pending requests */ + auth_request_handler_deinit(); + /* there are no more auth requests */ + auths_free(); auth_client_connections_deinit(); auth_master_connections_deinit(); - auth_worker_server_deinit(); - auth_request_handler_deinit(); - /* there may still be some async auth requests left, but above - functions should have marked all of them as destroyed. pass/userdb - deinits should abort the pending requests, which still triggers - callbacks, which should avoid crashes by checking the destroyed - state. */ - auths_deinit(); - passdb_cache_deinit(); + if (auth_worker_client != NULL) + auth_worker_client_destroy(&auth_worker_client); mech_register_deinit(&mech_reg); mech_deinit(global_auth_settings); @@ -179,6 +176,7 @@ static void main_deinit(void) userdbs_deinit(); passdbs_deinit(); + passdb_cache_deinit(); password_schemes_deinit(); sql_drivers_deinit(); random_deinit(); diff --git a/src/auth/passdb.c b/src/auth/passdb.c index 7be58eeceb..a936e1eef0 100644 --- a/src/auth/passdb.c +++ b/src/auth/passdb.c @@ -11,6 +11,10 @@ static ARRAY_DEFINE(passdb_interfaces, struct passdb_module_interface *); static ARRAY_DEFINE(passdb_modules, struct passdb_module *); +static const struct passdb_module_interface passdb_iface_deinit = { + .name = "deinit" +}; + static struct passdb_module_interface *passdb_interface_find(const char *name) { struct passdb_module_interface *const *ifaces; @@ -230,6 +234,9 @@ void passdb_deinit(struct passdb_module *passdb) if (passdb->iface.deinit != NULL) passdb->iface.deinit(passdb); + + /* make sure passdb isn't accessed again */ + passdb->iface = passdb_iface_deinit; } void passdbs_generate_md5(unsigned char md5[MD5_RESULTLEN]) diff --git a/src/auth/userdb.c b/src/auth/userdb.c index 2fa1f3a5cc..5563dd96b6 100644 --- a/src/auth/userdb.c +++ b/src/auth/userdb.c @@ -12,6 +12,10 @@ static ARRAY_DEFINE(userdb_interfaces, struct userdb_module_interface *); static ARRAY_DEFINE(userdb_modules, struct userdb_module *); +static const struct userdb_module_interface userdb_iface_deinit = { + .name = "deinit" +}; + static struct userdb_module_interface *userdb_interface_find(const char *name) { struct userdb_module_interface *const *ifaces; @@ -171,6 +175,9 @@ void userdb_deinit(struct userdb_module *userdb) if (userdb->iface->deinit != NULL) userdb->iface->deinit(userdb); + + /* make sure userdb isn't accessed again */ + userdb->iface = &userdb_iface_deinit; } void userdbs_generate_md5(unsigned char md5[MD5_RESULTLEN])