From 65e0c9b91b46e2dfb4388c8c5c1bc76dd9f8fbd8 Mon Sep 17 00:00:00 2001 From: Alexey Botchkov Date: Sat, 15 Jun 2019 01:02:55 +0400 Subject: MDEV-18661 loading the audit plugin causes performance regression. Plugin fixed to not lock the LOCK_operations when not active. Server fixed to lock the LOCK_plugin less - do it once per thread and then only if a plugin was installed/uninstalled. --- plugin/server_audit/server_audit.c | 112 ++++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 44 deletions(-) (limited to 'plugin') diff --git a/plugin/server_audit/server_audit.c b/plugin/server_audit/server_audit.c index c9e7e3a532a..3da52643787 100644 --- a/plugin/server_audit/server_audit.c +++ b/plugin/server_audit/server_audit.c @@ -15,7 +15,7 @@ #define PLUGIN_VERSION 0x104 -#define PLUGIN_STR_VERSION "1.4.4" +#define PLUGIN_STR_VERSION "1.4.7" #define _my_thread_var loc_thread_var @@ -295,7 +295,7 @@ static unsigned long long file_rotate_size; static unsigned int rotations; static my_bool rotate= TRUE; static char logging; -static int internal_stop_logging= 0; +static volatile int internal_stop_logging= 0; static char incl_user_buffer[1024]; static char excl_user_buffer[1024]; static char *big_buffer= NULL; @@ -533,16 +533,20 @@ static struct st_mysql_show_var audit_status[]= #if defined(HAVE_PSI_INTERFACE) && !defined(FLOGGER_NO_PSI) /* These belong to the service initialization */ static PSI_mutex_key key_LOCK_operations; +static PSI_mutex_key key_LOCK_atomic; static PSI_mutex_key key_LOCK_bigbuffer; static PSI_mutex_info mutex_key_list[]= { { &key_LOCK_operations, "SERVER_AUDIT_plugin::lock_operations", PSI_FLAG_GLOBAL}, + { &key_LOCK_atomic, "SERVER_AUDIT_plugin::lock_atomic", + PSI_FLAG_GLOBAL}, { &key_LOCK_bigbuffer, "SERVER_AUDIT_plugin::lock_bigbuffer", PSI_FLAG_GLOBAL} }; #endif static mysql_mutex_t lock_operations; +static mysql_mutex_t lock_atomic; static mysql_mutex_t lock_bigbuffer; /* The Percona server and partly MySQL don't support */ @@ -553,6 +557,14 @@ static mysql_mutex_t lock_bigbuffer; /* worths doing. */ #define CLIENT_ERROR if (!started_mysql) my_printf_error +#define ADD_ATOMIC(x, a) \ + do { \ + flogger_mutex_lock(&lock_atomic); \ + x+= a; \ + flogger_mutex_unlock(&lock_atomic); \ + } while (0) + + static uchar *getkey_user(const char *entry, size_t *length, my_bool nu __attribute__((unused)) ) { @@ -731,20 +743,20 @@ static int user_coll_fill(struct user_coll *c, char *users, if (cmp_user && take_over_cmp) { - internal_stop_logging= 1; + ADD_ATOMIC(internal_stop_logging, 1); CLIENT_ERROR(1, "User '%.*s' was removed from the" " server_audit_excl_users.", MYF(ME_JUST_WARNING), (int) cmp_length, users); - internal_stop_logging= 0; + ADD_ATOMIC(internal_stop_logging, -1); blank_user(cmp_user); refill_cmp_coll= 1; } else if (cmp_user) { - internal_stop_logging= 1; + ADD_ATOMIC(internal_stop_logging, 1); CLIENT_ERROR(1, "User '%.*s' is in the server_audit_incl_users, " "so wasn't added.", MYF(ME_JUST_WARNING), (int) cmp_length, users); - internal_stop_logging= 0; + ADD_ATOMIC(internal_stop_logging, -1); remove_user(users); continue; } @@ -1252,23 +1264,30 @@ static void change_connection(struct connection_info *cn, event->ip, event->ip_length); } -static int write_log(const char *message, int len) +static int write_log(const char *message, size_t len, int take_lock) { + int result= 0; + if (take_lock) + flogger_mutex_lock(&lock_operations); + if (output_type == OUTPUT_FILE) { if (logfile && - (is_active= (logger_write(logfile, message, len) == len))) - return 0; + (is_active= (logger_write(logfile, message, len) == (int) len))) + goto exit; ++log_write_failures; - return 1; + result= 1; } else if (output_type == OUTPUT_SYSLOG) { syslog(syslog_facility_codes[syslog_facility] | syslog_priority_codes[syslog_priority], - "%s %.*s", syslog_info, len, message); + "%s %.*s", syslog_info, (int) len, message); } - return 0; +exit: + if (take_lock) + flogger_mutex_unlock(&lock_operations); + return result; } @@ -1327,7 +1346,7 @@ static int log_connection(const struct connection_info *cn, csize+= my_snprintf(message+csize, sizeof(message) - 1 - csize, ",%.*s,,%d", cn->db_length, cn->db, event->status); message[csize]= '\n'; - return write_log(message, csize + 1); + return write_log(message, csize + 1, 1); } @@ -1348,7 +1367,7 @@ static int log_connection_event(const struct mysql_event_connection *event, csize+= my_snprintf(message+csize, sizeof(message) - 1 - csize, ",%.*s,,%d", event->database_length, event->database, event->status); message[csize]= '\n'; - return write_log(message, csize + 1); + return write_log(message, csize + 1, 1); } @@ -1477,21 +1496,28 @@ no_password: -static int do_log_user(const char *name) +static int do_log_user(const char *name, int take_lock) { size_t len; + int result; if (!name) return 0; len= strlen(name); - if (incl_user_coll.n_users) - return coll_search(&incl_user_coll, name, len) != 0; + if (take_lock) + flogger_mutex_lock(&lock_operations); - if (excl_user_coll.n_users) - return coll_search(&excl_user_coll, name, len) == 0; + if (incl_user_coll.n_users) + result= coll_search(&incl_user_coll, name, len) != 0; + else if (excl_user_coll.n_users) + result= coll_search(&excl_user_coll, name, len) == 0; + else + result= 1; - return 1; + if (take_lock) + flogger_mutex_unlock(&lock_operations); + return result; } @@ -1588,7 +1614,7 @@ not_in_list: static int log_statement_ex(const struct connection_info *cn, time_t ev_time, unsigned long thd_id, const char *query, unsigned int query_len, - int error_code, const char *type) + int error_code, const char *type, int take_lock) { size_t csize; char message_loc[1024]; @@ -1736,7 +1762,7 @@ do_log_query: csize+= my_snprintf(message+csize, message_size - 1 - csize, "\',%d", error_code); message[csize]= '\n'; - result= write_log(message, csize + 1); + result= write_log(message, csize + 1, take_lock); if (message == big_buffer) flogger_mutex_unlock(&lock_bigbuffer); @@ -1750,7 +1776,7 @@ static int log_statement(const struct connection_info *cn, { return log_statement_ex(cn, event->general_time, event->general_thread_id, event->general_query, event->general_query_length, - event->general_error_code, type); + event->general_error_code, type, 1); } @@ -1772,7 +1798,7 @@ static int log_table(const struct connection_info *cn, ",%.*s,%.*s,",event->database_length, event->database, event->table_length, event->table); message[csize]= '\n'; - return write_log(message, csize + 1); + return write_log(message, csize + 1, 1); } @@ -1796,7 +1822,7 @@ static int log_rename(const struct connection_info *cn, event->new_database_length, event->new_database, event->new_table_length, event->new_table); message[csize]= '\n'; - return write_log(message, csize + 1); + return write_log(message, csize + 1, 1); } @@ -1988,8 +2014,6 @@ void auditing(MYSQL_THD thd, unsigned int event_class, const void *ev) if (!thd || internal_stop_logging) return; - flogger_mutex_lock(&lock_operations); - if (maria_55_started && debug_server_started && event_class == MYSQL_AUDIT_GENERAL_CLASS) { @@ -2024,7 +2048,7 @@ void auditing(MYSQL_THD thd, unsigned int event_class, const void *ev) goto exit_func; if (event_class == MYSQL_AUDIT_GENERAL_CLASS && FILTER(EVENT_QUERY) && - cn && do_log_user(cn->user)) + cn && do_log_user(cn->user, 1)) { const struct mysql_event_general *event = (const struct mysql_event_general *) ev; @@ -2043,7 +2067,7 @@ void auditing(MYSQL_THD thd, unsigned int event_class, const void *ev) { const struct mysql_event_table *event = (const struct mysql_event_table *) ev; - if (do_log_user(event->user)) + if (do_log_user(event->user, 1)) { switch (event->event_subclass) { @@ -2109,7 +2133,6 @@ exit_func: } if (cn) cn->log_always= 0; - flogger_mutex_unlock(&lock_operations); } @@ -2377,6 +2400,7 @@ static int server_audit_init(void *p __attribute__((unused))) PSI_server->register_mutex("server_audit", mutex_key_list, 1); #endif flogger_mutex_init(key_LOCK_operations, &lock_operations, MY_MUTEX_INIT_FAST); + flogger_mutex_init(key_LOCK_operations, &lock_atomic, MY_MUTEX_INIT_FAST); flogger_mutex_init(key_LOCK_operations, &lock_bigbuffer, MY_MUTEX_INIT_FAST); coll_init(&incl_user_coll); @@ -2464,6 +2488,7 @@ static int server_audit_deinit(void *p __attribute__((unused))) (void) free(big_buffer); flogger_mutex_destroy(&lock_operations); + flogger_mutex_destroy(&lock_atomic); flogger_mutex_destroy(&lock_bigbuffer); error_header(); @@ -2553,10 +2578,10 @@ static void log_current_query(MYSQL_THD thd) return; cn= get_loc_info(thd); if (!ci_needs_setup(cn) && cn->query_length && - FILTER(EVENT_QUERY) && do_log_user(cn->user)) + FILTER(EVENT_QUERY) && do_log_user(cn->user, 0)) { log_statement_ex(cn, cn->query_time, thd_get_thread_id(thd), - cn->query, cn->query_length, 0, "QUERY"); + cn->query, cn->query_length, 0, "QUERY", 0); cn->log_always= 1; } } @@ -2568,12 +2593,13 @@ static void update_file_path(MYSQL_THD thd, { char *new_name= (*(char **) save) ? *(char **) save : empty_str; - if (!maria_55_started || !debug_server_started) - flogger_mutex_lock(&lock_operations); - internal_stop_logging= 1; + ADD_ATOMIC(internal_stop_logging, 1); error_header(); fprintf(stderr, "Log file name was changed to '%s'.\n", new_name); + if (!maria_55_started || !debug_server_started) + flogger_mutex_lock(&lock_operations); + if (logging) log_current_query(thd); @@ -2582,7 +2608,6 @@ static void update_file_path(MYSQL_THD thd, char *sav_path= file_path; file_path= new_name; - internal_stop_logging= 1; stop_logging(); if (start_logging()) { @@ -2598,16 +2623,15 @@ static void update_file_path(MYSQL_THD thd, } goto exit_func; } - internal_stop_logging= 0; } strncpy(path_buffer, new_name, sizeof(path_buffer)-1); path_buffer[sizeof(path_buffer)-1]= 0; file_path= path_buffer; exit_func: - internal_stop_logging= 0; if (!maria_55_started || !debug_server_started) flogger_mutex_unlock(&lock_operations); + ADD_ATOMIC(internal_stop_logging, -1); } @@ -2692,8 +2716,8 @@ static void update_output_type(MYSQL_THD thd, if (output_type == new_output_type) return; + ADD_ATOMIC(internal_stop_logging, 1); flogger_mutex_lock(&lock_operations); - internal_stop_logging= 1; if (logging) { log_current_query(thd); @@ -2707,8 +2731,8 @@ static void update_output_type(MYSQL_THD thd, if (logging) start_logging(); - internal_stop_logging= 0; flogger_mutex_unlock(&lock_operations); + ADD_ATOMIC(internal_stop_logging, -1); } @@ -2756,9 +2780,9 @@ static void update_logging(MYSQL_THD thd, if (new_logging == logging) return; + ADD_ATOMIC(internal_stop_logging, 1); if (!maria_55_started || !debug_server_started) flogger_mutex_lock(&lock_operations); - internal_stop_logging= 1; if ((logging= new_logging)) { start_logging(); @@ -2773,9 +2797,9 @@ static void update_logging(MYSQL_THD thd, stop_logging(); } - internal_stop_logging= 0; if (!maria_55_started || !debug_server_started) flogger_mutex_unlock(&lock_operations); + ADD_ATOMIC(internal_stop_logging, -1); } @@ -2787,16 +2811,16 @@ static void update_mode(MYSQL_THD thd __attribute__((unused)), if (mode_readonly || new_mode == mode) return; + ADD_ATOMIC(internal_stop_logging, 1); if (!maria_55_started || !debug_server_started) flogger_mutex_lock(&lock_operations); - internal_stop_logging= 1; mark_always_logged(thd); error_header(); fprintf(stderr, "Logging mode was changed from %d to %d.\n", mode, new_mode); mode= new_mode; - internal_stop_logging= 0; if (!maria_55_started || !debug_server_started) flogger_mutex_unlock(&lock_operations); + ADD_ATOMIC(internal_stop_logging, -1); } -- cgit v1.2.1 From d1fa6ba845aeaae5e4d8165bd6d7aeeb75d919b6 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Sat, 8 Jun 2019 17:36:52 +0200 Subject: MDEV-18328: Make DISKS plugin check some privilege to access information_schema.DISKS table Check FILE privelege. --- plugin/disks/information_schema_disks.cc | 10 ++++++--- .../mysql-test/disks/disks_notembedded.result | 15 +++++++++++++ .../disks/mysql-test/disks/disks_notembedded.test | 25 ++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 plugin/disks/mysql-test/disks/disks_notembedded.result create mode 100644 plugin/disks/mysql-test/disks/disks_notembedded.test (limited to 'plugin') diff --git a/plugin/disks/information_schema_disks.cc b/plugin/disks/information_schema_disks.cc index 7f7f4f06711..c4b558ba107 100644 --- a/plugin/disks/information_schema_disks.cc +++ b/plugin/disks/information_schema_disks.cc @@ -19,6 +19,7 @@ #include #include #include +#include /* check_global_access() */ bool schema_table_store_record(THD *thd, TABLE *table); @@ -83,6 +84,9 @@ int disks_fill_table(THD* pThd, TABLE_LIST* pTables, Item* pCond) int rv = 1; TABLE* pTable = pTables->table; + if (check_global_access(pThd, FILE_ACL, true)) + return 0; + FILE* pFile = setmntent("/etc/mtab", "r"); if (pFile) @@ -144,11 +148,11 @@ maria_declare_plugin(disks) PLUGIN_LICENSE_GPL, /* license type */ disks_table_init, /* init function */ NULL, /* deinit function */ - 0x0100, /* version = 1.0 */ + 0x0101, /* version = 1.1 */ NULL, /* no status variables */ NULL, /* no system variables */ - "1.0", /* String version representation */ - MariaDB_PLUGIN_MATURITY_BETA /* Maturity (see include/mysql/plugin.h)*/ + "1.1", /* String version representation */ + MariaDB_PLUGIN_MATURITY_STABLE /* Maturity (see include/mysql/plugin.h)*/ } mysql_declare_plugin_end; diff --git a/plugin/disks/mysql-test/disks/disks_notembedded.result b/plugin/disks/mysql-test/disks/disks_notembedded.result new file mode 100644 index 00000000000..f0f9e46aaca --- /dev/null +++ b/plugin/disks/mysql-test/disks/disks_notembedded.result @@ -0,0 +1,15 @@ +# +# MDEV-18328: Make DISKS plugin check some privilege to access +# information_schema.DISKS table +# +CREATE USER user1@localhost; +GRANT SELECT ON *.* TO user1@localhost; +select sum(Total) > sum(Available), sum(Total)>sum(Used) from information_schema.disks; +sum(Total) > sum(Available) sum(Total)>sum(Used) +NULL NULL +GRANT FILE ON *.* TO user1@localhost; +select sum(Total) > sum(Available), sum(Total)>sum(Used) from information_schema.disks; +sum(Total) > sum(Available) sum(Total)>sum(Used) +1 1 +DROP USER user1@localhost; +# End of 10.1 tests diff --git a/plugin/disks/mysql-test/disks/disks_notembedded.test b/plugin/disks/mysql-test/disks/disks_notembedded.test new file mode 100644 index 00000000000..a0f6c2e5887 --- /dev/null +++ b/plugin/disks/mysql-test/disks/disks_notembedded.test @@ -0,0 +1,25 @@ +source include/not_embedded.inc; + +--echo # +--echo # MDEV-18328: Make DISKS plugin check some privilege to access +--echo # information_schema.DISKS table +--echo # + +CREATE USER user1@localhost; +GRANT SELECT ON *.* TO user1@localhost; + +connect (con1,localhost,user1,,); +connection con1; +select sum(Total) > sum(Available), sum(Total)>sum(Used) from information_schema.disks; +disconnect con1; + +connection default; +GRANT FILE ON *.* TO user1@localhost; + +connect (con1,localhost,user1,,); +connection con1; +select sum(Total) > sum(Available), sum(Total)>sum(Used) from information_schema.disks; +connection default; +DROP USER user1@localhost; + +--echo # End of 10.1 tests -- cgit v1.2.1