Skip to content

Commit

Permalink
[plugin] PS-269: Initial Percona Server 8.0.12 tree
Browse files Browse the repository at this point in the history
PS-5741: Incorrect use of memset_s in keyring_vault.

Fixed the usage of memset_s. The arguments should be:
void memset_s(void *dest, size_t dest_max, int c, size_t n)
where the 2nd argument is size of buffer and the 3rd is
argument is character to fill.

---------------------------------------------------------------------------

PS-7769 - Fix use-after-return error in audit_log_exclude_accounts_validate

---

*Problem:*

`st_mysql_value::val_str` might return a pointer to `buf` which after
the function called is deleted. Therefore the value in `save`, after
reuturnin from the function, is invalid.

In this particular case, the error is not manifesting as val_str`
returns memory allocated with `thd_strmake` and it does not use `buf`.

*Solution:*

Allocate memory with `thd_strmake` so the memory in `save` is not local.

---------------------------------------------------------------------------

Fix test main.bug12969156 when WITH_ASAN=ON

*Problem:*

ASAN complains about stack-buffer-overflow on function `mysql_heartbeat`:

```
==90890==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fe746d06d14 at pc 0x7fe760f5b017 bp 0x7fe746d06cd0 sp 0x7fe746d06478
WRITE of size 24 at 0x7fe746d06d14 thread T16777215

Address 0x7fe746d06d14 is located in stack of thread T26 at offset 340 in frame
    #0 0x7fe746d0a55c in mysql_heartbeat(void*) /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:62

  This frame has 4 object(s):
    [48, 56) 'result' (line 66)
    [80, 112) '_db_stack_frame_' (line 63)
    [144, 200) 'tm_tmp' (line 67)
    [240, 340) 'buffer' (line 65) <== Memory access at offset 340 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
Thread T26 created by T25 here:
    #0 0x7fe760f5f6d5 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
    #1 0x557ccbbcb857 in my_thread_create /home/yura/ws/percona-server/mysys/my_thread.c:104
    #2 0x7fe746d0b21a in daemon_example_plugin_init /home/yura/ws/percona-server/plugin/daemon_example/daemon_example.cc:148
    #3 0x557ccb4c69c7 in plugin_initialize /home/yura/ws/percona-server/sql/sql_plugin.cc:1279
    #4 0x557ccb4d19cd in mysql_install_plugin /home/yura/ws/percona-server/sql/sql_plugin.cc:2279
    #5 0x557ccb4d218f in Sql_cmd_install_plugin::execute(THD*) /home/yura/ws/percona-server/sql/sql_plugin.cc:4664
    #6 0x557ccb47695e in mysql_execute_command(THD*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5160
    #7 0x557ccb47977c in mysql_parse(THD*, Parser_state*, bool) /home/yura/ws/percona-server/sql/sql_parse.cc:5952
    #8 0x557ccb47b6c2 in dispatch_command(THD*, COM_DATA const*, enum_server_command) /home/yura/ws/percona-server/sql/sql_parse.cc:1544
    #9 0x557ccb47de1d in do_command(THD*) /home/yura/ws/percona-server/sql/sql_parse.cc:1065
    #10 0x557ccb6ac294 in handle_connection /home/yura/ws/percona-server/sql/conn_handler/connection_handler_per_thread.cc:325
    #11 0x557ccbbfabb0 in pfs_spawn_thread /home/yura/ws/percona-server/storage/perfschema/pfs.cc:2198
    #12 0x7fe760ab544f in start_thread nptl/pthread_create.c:473
```

The reason is that `my_thread_cancel` is used to finish the daemon thread. This is not and orderly way of finishing the thread. ASAN does not register the stack variables are not used anymore which generates the error above.

This is a benign error as all the variables are on the stack.

*Solution*:

Finish the thread in orderly way by using a signalling variable.

---------------------------------------------------------------------------

PS-8204: Fix XML escape rules for audit plugin

https://jira.percona.com/browse/PS-8204

There was a wrong length specified for some XML
escape rules. As a result of this terminating null symbol from
replacement rule was copied into resulting string. This lead to
quer text truncation in audit log file.
In addition added empty replacement rules for '\b' and 'f' symbols
which just remove them from resulting string. These symboles are
not supported in XML 1.0.
  • Loading branch information
inikep committed Aug 23, 2023
1 parent 3da481e commit d908650
Show file tree
Hide file tree
Showing 55 changed files with 3,907 additions and 1,576 deletions.
4 changes: 2 additions & 2 deletions include/my_bitmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ extern uint bitmap_bits_set(const MY_BITMAP *map);
extern void bitmap_free(MY_BITMAP *map);
extern void bitmap_set_above(MY_BITMAP *map, uint from_byte, bool use_bit);
extern void bitmap_set_prefix(MY_BITMAP *map, uint prefix_size);
extern void bitmap_intersect(MY_BITMAP *to, const MY_BITMAP *from);
extern void bitmap_intersect(MY_BITMAP *map, const MY_BITMAP *map2);
extern void bitmap_subtract(MY_BITMAP *map, const MY_BITMAP *map2);
extern void bitmap_union(MY_BITMAP *map, const MY_BITMAP *map2);
extern void bitmap_xor(MY_BITMAP *map, const MY_BITMAP *map2);
Expand Down Expand Up @@ -135,4 +135,4 @@ static inline void bitmap_set_all(MY_BITMAP *map) {
memset(map->bitmap, 0xFF, 4 * no_words_in_map(map));
}

#endif // MY_BITMAP_INCLUDED
#endif /* MY_BITMAP_INCLUDED */
3 changes: 3 additions & 0 deletions include/my_sys.h
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,9 @@ extern size_t my_fwrite(FILE *stream, const uchar *Buffer, size_t Count,
myf MyFlags);
extern my_off_t my_fseek(FILE *stream, my_off_t pos, int whence);
extern my_off_t my_ftell(FILE *stream);
#if !defined(HAVE_MEMSET_S)
void memset_s(void *dest, size_t dest_max, int c, size_t n);
#endif

/* implemented in my_syslog.c */

Expand Down
9 changes: 9 additions & 0 deletions mysql-test/include/plugin.defs
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,12 @@ component_test_event_tracking_consumer plugin_output_directory no COMP

# component_test_event_tracking_producer_a
component_test_event_tracking_producer_a plugin_output_directory no COMPONENT_TEST_EVENT_TRACKING_PRODUCER_A

# Percona additions
auth_socket plugin_output_directory no SOCKET_AUTH
ha_tokudb plugin_output_directory no TOKUDB tokudb,tokudb_trx,tokudb_locks,tokudb_lock_waits,tokudb_fractal_tree_info,tokudb_background_job_status,tokudb_file_map
tokudb_backup plugin_output_directory no TOKUDB_BACKUP tokudb_backup
ha_rocksdb plugin_output_directory no ROCKSDB rocksdb,rocksdb_cfstats,rocksdb_dbstats,rocksdb_perf_context,rocksdb_perf_context_global,rocksdb_cf_options,rocksdb_compaction_history,rocksdb_compaction_stats,rocksdb_active_compaction_stats,rocksdb_global_info,rocksdb_ddl,rocksdb_index_file_map,rocksdb_locks,rocksdb_trx,rocksdb_deadlock,rocksdb_sst_props,rocksdb_live_files_metadata
auth_pam plugin_output_directory no AUTH_PAM
auth_pam_compat plugin_output_directory no AUTH_PAM_COMPAT
keyring_vault plugin_output_directory no KEYRING_VAULT_PLUGIN keyring_vault
2 changes: 2 additions & 0 deletions mysys/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ SET(MY_TIME_SOURCES my_time.cc my_systime.cc)

ADD_CONVENIENCE_LIBRARY(mytime ${MY_TIME_SOURCES})

INCLUDE_DIRECTORIES(SYSTEM ${BOOST_PATCHES_DIR} ${BOOST_INCLUDE_DIR})

SET(MYSYS_SOURCES
array.cc
charset.cc
Expand Down
13 changes: 13 additions & 0 deletions mysys/my_malloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -561,3 +561,16 @@ char *my_strndup(PSI_memory_key key, const char *from, size_t length,
}
return ptr;
}

#if !defined(HAVE_MEMSET_S)
void memset_s(void *dest, size_t dest_max, int c, size_t n) {
#if defined(WIN32)
SecureZeroMemory(dest, n);
#else
volatile unsigned char *p = static_cast<unsigned char *>(dest);
while (dest_max-- && n--) {
*p++ = c;
}
#endif
}
#endif
7 changes: 5 additions & 2 deletions plugin/daemon_example/daemon_example.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <atomic>

#include "m_string.h" // strlen
#include "my_dbug.h"
Expand Down Expand Up @@ -62,6 +63,7 @@ static void init_deamon_example_psi_keys() {
struct mysql_heartbeat_context {
my_thread_handle heartbeat_thread;
File heartbeat_file;
std::atomic_bool done;
};

static void *mysql_heartbeat(void *p) {
Expand All @@ -71,7 +73,7 @@ static void *mysql_heartbeat(void *p) {
time_t result;
struct tm tm_tmp;

while (true) {
while (!con->done.load()) {
sleep(5);

result = time(nullptr);
Expand Down Expand Up @@ -124,6 +126,7 @@ static int daemon_example_plugin_init(void *p) {
MY_REPLACE_EXT | MY_UNPACK_FILENAME);
unlink(heartbeat_filename);
con->heartbeat_file = my_open(heartbeat_filename, O_CREAT | O_RDWR, MYF(0));
con->done.store(false);

/*
No threads exist at this point in time, so this is thread safe.
Expand Down Expand Up @@ -172,7 +175,7 @@ static int daemon_example_plugin_deinit(void *p) {
struct tm tm_tmp;
void *dummy_retval;

my_thread_cancel(&con->heartbeat_thread);
con->done.store(true);

localtime_r(&result, &tm_tmp);
snprintf(buffer, sizeof(buffer),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@

#include "openssl/engine.h"

#if OPENSSL_VERSION_NUMBER >= 0x30000000L
#include <openssl/evp.h>
#include <openssl/provider.h>
#endif

#include "xcom/task_debug.h"
#include "xcom/x_platform.h"

Expand Down
74 changes: 35 additions & 39 deletions plugin/innodb_memcached/daemon_memcached/daemon/memcached_mysql.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ static SYS_VAR *daemon_memcached_sys_var[] = {
THD *thd_get_current_thd(); // from sql_class.cc

static void emit_deprecation_message() {
push_deprecated_warn_no_replacement(thd_get_current_thd(),
"InnoDB Memcached Plugin");
push_deprecated_warn_no_replacement(thd_get_current_thd(),
"InnoDB Memcached Plugin");
}

static int daemon_memcached_plugin_deinit(void *p)
Expand Down Expand Up @@ -161,49 +161,45 @@ static int daemon_memcached_plugin_init(void *p)
pthread_attr_t attr;
struct st_plugin_int* plugin = (struct st_plugin_int *)p;

emit_deprecation_message();
emit_deprecation_message();

con = (mysql_memcached_context*) my_malloc(PSI_INSTRUMENT_ME,
con = (mysql_memcached_context *)my_malloc(PSI_INSTRUMENT_ME,
sizeof(*con), MYF(0));

if (mci_engine_library) {
char* lib_path = (mci_eng_lib_path)
? mci_eng_lib_path : opt_plugin_dir;
int lib_len = strlen(lib_path)
+ strlen(mci_engine_library)
+ strlen(FN_DIRSEP) + 1;

con->memcached_conf.m_engine_library = (char*) my_malloc(
PSI_INSTRUMENT_ME,
lib_len, MYF(0));

strxmov(con->memcached_conf.m_engine_library, lib_path,
FN_DIRSEP, mci_engine_library, NullS);
} else {
con->memcached_conf.m_engine_library = NULL;
}
if (mci_engine_library) {
char *lib_path =
(mci_eng_lib_path) ? mci_eng_lib_path : opt_plugin_dir;
int lib_len = strlen(lib_path) + strlen(mci_engine_library) +
strlen(FN_DIRSEP) + 1;

con->memcached_conf.m_mem_option = mci_memcached_option;
con->memcached_conf.m_innodb_api_cb = plugin->data;
con->memcached_conf.m_r_batch_size = mci_r_batch_size;
con->memcached_conf.m_w_batch_size = mci_w_batch_size;
con->memcached_conf.m_enable_binlog = mci_enable_binlog;

pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);

/* now create the thread */
if (pthread_create(&con->memcached_thread, &attr,
daemon_memcached_main,
(void *)&con->memcached_conf) != 0)
{
fprintf(stderr,"Could not create memcached daemon thread!\n");
exit(0);
}
con->memcached_conf.m_engine_library =
(char *)my_malloc(PSI_INSTRUMENT_ME, lib_len, MYF(0));

plugin->data= (void *)con;
strxmov(con->memcached_conf.m_engine_library, lib_path, FN_DIRSEP,
mci_engine_library, NullS);
} else {
con->memcached_conf.m_engine_library = NULL;
}

return(0);
con->memcached_conf.m_mem_option = mci_memcached_option;
con->memcached_conf.m_innodb_api_cb = plugin->data;
con->memcached_conf.m_r_batch_size = mci_r_batch_size;
con->memcached_conf.m_w_batch_size = mci_w_batch_size;
con->memcached_conf.m_enable_binlog = mci_enable_binlog;

pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);

/* now create the thread */
if (pthread_create(&con->memcached_thread, &attr, daemon_memcached_main,
(void *)&con->memcached_conf) != 0) {
fprintf(stderr, "Could not create memcached daemon thread!\n");
exit(0);
}

plugin->data = (void *)con;

return (0);
}

struct st_mysql_daemon daemon_memcached_plugin =
Expand Down
23 changes: 0 additions & 23 deletions plugin/innodb_memcached/daemon_memcached/daemon/topkeys.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,29 +122,6 @@ topkey_item_t *topkeys_item_get_or_create(topkeys_t *tk, const void *key, size_t
return item;
}

static inline void append_stat(const void *cookie,
const char *name,
size_t namelen,
const char *key,
size_t nkey,
int value,
ADD_STAT add_stats) {
char key_str[128];
char val_str[128];
int klen, vlen;

klen = sizeof(key_str) - namelen - 2;
if (nkey < klen) {
klen = nkey;
}
memcpy(key_str, key, klen);
key_str[klen] = '.';
memcpy(&key_str[klen+1], name, namelen + 1);
klen += namelen + 1;
vlen = snprintf(val_str, sizeof(val_str) - 1, "%d", value);
add_stats(key_str, klen, val_str, vlen, cookie);
}

struct tk_context {
const void *cookie;
ADD_STAT add_stat;
Expand Down
3 changes: 3 additions & 0 deletions plugin/innodb_memcached/innodb_memcache/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ INCLUDE_DIRECTORIES(
IF(CMAKE_C_FLAGS MATCHES "-Werror")
STRING(REGEX REPLACE "-Werror( |$)" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
ENDIF(CMAKE_C_FLAGS MATCHES "-Werror")
IF(CMAKE_CXX_FLAGS MATCHES "-Werror")
STRING(REGEX REPLACE "-Werror( |$)" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
ENDIF(CMAKE_CXX_FLAGS MATCHES "-Werror")

# Set extra flags for the C/CXX compiler
IF(MY_COMPILER_IS_GNU_OR_CLANG)
Expand Down
2 changes: 1 addition & 1 deletion plugin/keyring/buffered_file_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ bool Buffered_file_io::check_if_keyring_file_can_be_opened_or_created() {
@retval true - there was an error with initializing keyring file
@retval false - keyring file has been initialized successfully
*/
bool Buffered_file_io::init(std::string *keyring_filename) {
bool Buffered_file_io::init(const std::string *keyring_filename) {
// file name can't be empty
assert(keyring_filename->empty() == false);

Expand Down
2 changes: 1 addition & 1 deletion plugin/keyring/buffered_file_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Buffered_file_io : public IKeyring_io {

// ================= IKeyring_io implementation ================= //

bool init(std::string *keyring_filename) override;
bool init(const std::string *keyring_filename) override;
bool flush_to_backup(ISerialized_object *serialized_object) override;
bool flush_to_storage(ISerialized_object *serialized_object) override;
ISerializer *get_serializer() override;
Expand Down
2 changes: 1 addition & 1 deletion plugin/keyring/common/i_keyring_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace keyring {

class IKeyring_io : public Keyring_alloc {
public:
virtual bool init(std::string *keyring_storage_url) = 0;
virtual bool init(const std::string *keyring_storage_url) = 0;
virtual bool flush_to_backup(ISerialized_object *serialized_object) = 0;
virtual bool flush_to_storage(ISerialized_object *serialized_object) = 0;

Expand Down
1 change: 1 addition & 0 deletions plugin/keyring/common/i_keyring_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct IKey : public Keyring_alloc {
virtual size_t get_key_data_size() = 0;
virtual size_t get_key_pod_size() const = 0;
virtual uchar *release_key_data() = 0;
virtual void xor_data(uchar *data, size_t data_len) = 0;
virtual void xor_data() = 0;
virtual void set_key_data(uchar *key_data, size_t key_data_size) = 0;
virtual void set_key_type(const std::string *key_type) = 0;
Expand Down
12 changes: 8 additions & 4 deletions plugin/keyring/common/keyring_key.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,16 @@ size_t Key::get_key_pod_size() const {
return key_pod_size_aligned;
}

void Key::xor_data() {
if (key == nullptr) return;
void Key::xor_data(uchar *data, size_t data_len) {
static const char *obfuscate_str = "*305=Ljt0*!@$Hnm(*-9-w;:";
for (size_t i = 0, l = 0; i < key_len;
for (size_t i = 0, l = 0; i < data_len;
++i, l = ((l + 1) % strlen(obfuscate_str)))
key.get()[i] ^= obfuscate_str[l];
data[i] ^= obfuscate_str[l];
}

void Key::xor_data() {
if (key == nullptr) return;
xor_data(key.get(), key_len);
}

bool Key::is_key_id_valid() { return key_id.length() > 0; }
Expand Down
3 changes: 2 additions & 1 deletion plugin/keyring/common/keyring_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct Key : IKey {
size_t get_key_data_size() override;
size_t get_key_pod_size() const override;
uchar *release_key_data() override;
void xor_data(uchar *data, size_t data_len) override;
void xor_data() override;
void set_key_data(uchar *key_data, size_t key_data_size) override;
void set_key_type(const std::string *key_type) override;
Expand All @@ -68,7 +69,7 @@ struct Key : IKey {
const void *a_key, size_t a_key_len);

void clear_key_data();
void create_key_signature() const;
virtual void create_key_signature() const;
bool load_string_from_buffer(const uchar *buffer, size_t *buffer_position,
size_t key_pod_size, std::string *string,
size_t string_length);
Expand Down
42 changes: 41 additions & 1 deletion plugin/keyring/common/keyring_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@
#ifndef MYSQL_KEYRING_MEMORY_H
#define MYSQL_KEYRING_MEMORY_H

#include <mysql/plugin_keyring.h>
#include <string.h>
#include <limits>
#include <memory>

#include "my_sys.h"
#include "mysql/service_mysql_alloc.h"

namespace keyring {

extern PSI_memory_key key_memory_KEYRING;
Expand All @@ -48,6 +51,43 @@ class Keyring_alloc {
static void operator delete(void *ptr, std::size_t) { my_free(ptr); }
static void operator delete[](void *ptr, std::size_t) { my_free(ptr); }
};

template <class T>
class Secure_allocator {
public:
using value_type = T;

Secure_allocator() noexcept {}

template <class U>
Secure_allocator(const Secure_allocator<U> &) noexcept {}

T *allocate(size_t n) {
if (n == 0)
return nullptr;
else if (n > INT_MAX)
throw std::bad_alloc();
return keyring_malloc<T *>(n * sizeof(T));
}

void deallocate(T *p, size_t n) noexcept {
memset_s(p, n, 0, n);
my_free(p);
}
};

template <typename T1, typename T2>
bool operator==(const Secure_allocator<T1> &,
const Secure_allocator<T2> &) noexcept {
return true;
}

template <typename T1, typename T2>
bool operator!=(const Secure_allocator<T1> &,
const Secure_allocator<T2> &) noexcept {
return false;
}

} // namespace keyring

#endif // MYSQL_KEYRING_MEMORY_H
Loading

0 comments on commit d908650

Please sign in to comment.