From 9d9851d8e656ab17ff330c125bfc21c329a52383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Sun, 8 Aug 2021 19:48:26 +0200 Subject: [PATCH 1/8] Added ASAN (AddressSanitizer) compilation support to ProxySQL #3554 --- Makefile | 10 ++++++++++ deps/Makefile | 4 ++++ lib/Makefile | 11 ++++++++++- src/Makefile | 15 ++++++++++++++- test/tap/tests/Makefile | 18 ++++++++++++++---- 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index f3d6852dd..d751befd7 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,12 @@ endif ### NOTES: ### to compile without jemalloc, set environment variable NOJEMALLOC=1 ### to compile with gcov code coverage, set environment variable WITHGCOV=1 +### to compile with ASAN, set environment variables NOJEMALLOC=1, WITHASAN=1: +### * To perform a full ProxySQL build with ASAN then execute: +### +### ``` +### make build_deps_debug -j$(nproc) && make debug -j$(nproc) && make build_tap_test_debug -j$(nproc) +### ``` O0=-O0 O2=-O2 @@ -133,6 +139,10 @@ build_lib_testall: build_deps_debug build_tap_test: build_src cd test/tap && OPTZ="${O0} -ggdb -DDEBUG" CC=${CC} CXX=${CXX} ${MAKE} +.PHONY: build_tap_test_debug +build_tap_test_debug: build_src + cd test/tap && OPTZ="${O0} -ggdb -DDEBUG" CC=${CC} CXX=${CXX} ${MAKE} debug + .PHONY: build_src_debug build_src_debug: build_deps build_lib_debug cd src && OPTZ="${O0} -ggdb -DDEBUG" CC=${CC} CXX=${CXX} ${MAKE} diff --git a/deps/Makefile b/deps/Makefile index f5be0d551..a22f75463 100644 --- a/deps/Makefile +++ b/deps/Makefile @@ -165,6 +165,7 @@ jemalloc/jemalloc/lib/libjemalloc.a: jemalloc: jemalloc/jemalloc/lib/libjemalloc.a +WITHASAN := $(shell echo $(WITHASAN)) mariadb-client-library/mariadb_client/libmariadb/libmariadbclient.a: libssl/openssl/libssl.a cd mariadb-client-library && rm -rf mariadb-connector-c-3.1.9-src @@ -189,6 +190,9 @@ mariadb-client-library/mariadb_client/libmariadb/libmariadbclient.a: libssl/open cd mariadb-client-library/mariadb_client && patch -p0 < ../client_deprecate_eof.patch cd mariadb-client-library/mariadb_client && patch -p0 < ../cr_new_stmt_metadata_removal.patch cd mariadb-client-library/mariadb_client && patch -p0 < ../ps_buffer_stmt_read_all_rows.patch +ifeq ($(WITHASAN),1) + cd mariadb-client-library/mariadb_client && patch -p0 < ../mariadb_asan.patch +endif cd mariadb-client-library/mariadb_client && CC=${CC} CXX=${CXX} ${MAKE} mariadbclient # cd mariadb-client-library/mariadb_client/include && make my_config.h diff --git a/lib/Makefile b/lib/Makefile index e799ce8e5..1fd1c04ef 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -67,6 +67,15 @@ ODIR= obj #CXX=g++ #CC=clang +WITHASANVAR := $(shell echo $(WITHASAN)) +ifeq ($(WITHASANVAR),1) +WASAN=-fsanitize=address +# Force the disable of JEMALLOC, since ASAN isn't compatible. +export NOJEMALLOC = 1 +else +WASAN= +endif + #CFLAGS=$(IDIRS) $(OPTZ) $(DEBUG) -Wall #-lcrypto #CXXFLAGS=-std=c++11 $(CFLAGS) $(LDIRS) $(LIBS) NOJEMALLOC := $(shell echo $(NOJEMALLOC)) @@ -100,7 +109,7 @@ ifeq ($(UNAME_S),Darwin) endif -MYCFLAGS=$(IDIRS) $(OPTZ) $(DEBUG) -Wall -DGITVERSION=\"$(GIT_VERSION)\" $(NOJEM) $(WGCOV) +MYCFLAGS=$(IDIRS) $(OPTZ) $(DEBUG) -Wall -DGITVERSION=\"$(GIT_VERSION)\" $(NOJEM) $(WGCOV) $(WASAN) MYCXXFLAGS=-std=c++11 $(MYCFLAGS) $(PSQLCH) default: libproxysql.a diff --git a/src/Makefile b/src/Makefile index 48a860138..c0617045e 100644 --- a/src/Makefile +++ b/src/Makefile @@ -100,12 +100,25 @@ else WGCOV= endif -MYCXXFLAGS=-std=c++11 $(IDIRS) $(OPTZ) $(DEBUG) $(PSQLCH) -DGITVERSION=\"$(GIT_VERSION)\" $(WGCOV) +WITHASANVAR := $(shell echo $(WITHASAN)) +ifeq ($(WITHASANVAR),1) +WASAN= -fsanitize=address +# Force the disable of JEMALLOC, since ASAN isn't compatible. +export NOJEMALLOC = 1 +else +WASAN= +endif + +MYCXXFLAGS=-std=c++11 $(IDIRS) $(OPTZ) $(DEBUG) $(PSQLCH) -DGITVERSION=\"$(GIT_VERSION)\" $(WGCOV) $(WASAN) ifeq ($(WITHGCOVVAR),1) LDFLAGS+= -lgcov --coverage endif +ifeq ($(WITHASANVAR),1) +LDFLAGS+= -fsanitize=address +endif + NOJEMALLOC := $(shell echo $(NOJEMALLOC)) ifeq ($(NOJEMALLOC),1) MYLIBS=-Wl,--export-dynamic -Wl,-Bstatic -lconfig -lproxysql -ldaemon -lconfig++ -lre2 -lpcrecpp -lpcre -lmariadbclient -lhttpserver -lmicrohttpd -linjection -lcurl -lssl -lcrypto -lev -Wl,-Bdynamic -lgnutls -lpthread -lm -lz -lrt -lprometheus-cpp-pull -lprometheus-cpp-core $(EXTRALINK) diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index 9993439c6..8fd2e2de1 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -73,7 +73,7 @@ OBJ=../../../src/obj/proxysql_global.o ../../../src/obj/main.o INCLUDEDIRS=-I../tap -I$(RE2_PATH) -I$(IDIR) -I$(JEMALLOC_IDIR) -I$(SQLITE3_DIR) -I$(MICROHTTPD_IDIR) -I$(LIBHTTPSERVER_IDIR) -I$(CURL_IDIR) -I$(DAEMONPATH_IDIR) -I$(MARIADB_IDIR) -I$(SSL_IDIR) -I$(JSON_IDIR) -I$(LIBCONFIG_IDIR) -I$(PROMETHEUS_IDIR) -I$(EV_IDIR) LDIRS=-L$(TAP_LIBDIR) -L$(LDIR) -L$(JEMALLOC_LDIR) $(LIBCONFIG_LDIR) -L$(RE2_PATH)/obj -L$(MARIADB_LDIR) -L$(DAEMONPATH_LDIR) -L$(PCRE_LDIR) -L$(MICROHTTPD_LDIR) -L$(LIBHTTPSERVER_LDIR) -L$(LIBINJECTION_LDIR) -L$(CURL_LDIR) -L$(EV_LDIR) -L$(SSL_LDIR) -L$(PROMETHEUS_LDIR) -MYLIBS=-Wl,--export-dynamic -Wl,-Bstatic -lconfig -lproxysql -ldaemon -ljemalloc -lconfig++ -lre2 -lpcrecpp -lpcre -lmariadbclient -lhttpserver -lmicrohttpd -linjection -lcurl -lssl -lcrypto -lev -Wl,-Bdynamic -lgnutls -lpthread -lm -lz -lrt $(EXTRALINK) -lprometheus-cpp-pull -lprometheus-cpp-core +MYLIBS=-Wl,--export-dynamic -Wl,-Bstatic -lconfig -ldaemon -ljemalloc -lconfig++ -lre2 -lpcrecpp -lpcre -lmariadbclient -lhttpserver -lmicrohttpd -linjection -lcurl -lssl -lcrypto -lev -Wl,-Bdynamic -lgnutls -lpthread -lm -lz -lrt $(EXTRALINK) -lprometheus-cpp-pull -lprometheus-cpp-core STATIC_LIBS= $(SSL_LDIR)/libssl.a $(SSL_LDIR)/libcrypto.a .PHONY: all @@ -90,8 +90,15 @@ else WGCOV= endif -OPT=-O2 $(WGCOV) -Wl,--no-as-needed -debug: OPT=-O0 -DDEBUG $(WGCOV) -ggdb -Wl,--no-as-needed +WITHASANVAR := $(shell echo $(WITHASAN)) +ifeq ($(WITHASANVAR),1) +WASAN=-fsanitize=address +else +WASAN= +endif + +OPT=-O2 -Wl,--no-as-needed +debug: OPT=-O0 -DDEBUG -ggdb -Wl,--no-as-needed $(WGCOV) $(WASAN) debug: tests tests: $(patsubst %.cpp,%,$(wildcard *-t.cpp)) setparser_test @@ -113,6 +120,9 @@ aurora: aurora.cpp $(TAP_LIBDIR)/libtap.a test_tokenizer-t: test_tokenizer-t.cpp $(TAP_LIBDIR)/libtap.a g++ test_tokenizer-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(MYLIBS) -lproxysql -ltap -Wl,--no-as-needed -ldl -lpthread -o test_tokenizer-t -DGITVERSION=\"$(GIT_VERSION)\" +sqlite3-t: sqlite3-t.cpp $(TAP_LIBDIR)/libtap.a + g++ sqlite3-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 -lproxysql $(MYLIBS) -ltap -Wl,--no-as-needed -ldl -lpthread -o sqlite3-t -DGITVERSION=\"$(GIT_VERSION)\" + test_gtid_forwarding-t: test_gtid_forwarding-t.cpp $(TAP_LIBDIR)/libtap.a g++ test_gtid_forwarding-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(MYLIBS) -ltap -Wl,--no-as-needed -ldl -lpthread -o test_gtid_forwarding-t -DGITVERSION=\"$(GIT_VERSION)\" @@ -126,4 +136,4 @@ test_set_collation-t: test_set_collation-t.cpp $(TAP_LIBDIR)/libtap.a g++ test_set_collation-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(MYLIBS) -ltap -Wl,--no-as-needed -ldl -lpthread -o test_set_collation-t -DGITVERSION=\"$(GIT_VERSION)\" setparser_test: setparser_test.cpp $(TAP_LIBDIR)/libtap.a $(RE2_PATH)/util/test.cc $(LDIR)/set_parser.cpp $(LIBPROXYSQLAR) - g++ -DDEBUG setparser_test.cpp $(RE2_PATH)/util/test.cc ../../../src/obj/proxysql_global.o $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(MYLIBS) -ltap -ldl -lpthread -o setparser_test -DGITVERSION=\"$(GIT_VERSION)\" + g++ -DDEBUG setparser_test.cpp $(RE2_PATH)/util/test.cc ../../../src/obj/proxysql_global.o $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 -lproxysql $(MYLIBS) -ltap -ldl -lpthread $(WASAN) -o setparser_test -DGITVERSION=\"$(GIT_VERSION)\" From 0f8aa299ac9d8c4a10a21ae1e8be80f3d36cfd67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 19 Aug 2021 21:33:17 +0200 Subject: [PATCH 2/8] Fixed memory corruption reported by ASAN due to unexpected packet by 'mysqlsh' #3554 --- lib/MySQL_Protocol.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/MySQL_Protocol.cpp b/lib/MySQL_Protocol.cpp index cad2d1c29..bf7e766f5 100644 --- a/lib/MySQL_Protocol.cpp +++ b/lib/MySQL_Protocol.cpp @@ -1618,6 +1618,15 @@ bool MySQL_Protocol::process_pkt_handshake_response(unsigned char *pkt, unsigned //Copy4B(&hdr,pkt); pkt += sizeof(mysql_hdr); + // NOTE: 'mysqlsh' sends a 'COM_INIT_DB' as soon as the connection is openned + // before ProxySQL has sent 'Server Greeting' messsage. Because this packet is + // unexpected, we simple return 'false' and exit. + if (hdr.pkt_id == 0 && *pkt == 2) { + ret = false; + proxy_debug(PROXY_DEBUG_MYSQL_AUTH, 5, "Session=%p , DS=%p , user='%s' . Client is disconnecting\n", (*myds), (*myds)->sess, user); + goto __exit_process_pkt_handshake_response; + } + if ((*myds)->myconn->userinfo->username) { (*myds)->switching_auth_stage=2; if (len==5) { From 58460bd323fae96afd0dc838618122d630167bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 19 Aug 2021 21:37:17 +0200 Subject: [PATCH 3/8] Fixed 'heap-buffer-overflow' detected by ASAN in 'generate_show_fields_from' #3554 --- lib/ProxySQL_Admin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index ef20bdc7e..e29d97f31 100644 --- a/lib/ProxySQL_Admin.cpp +++ b/lib/ProxySQL_Admin.cpp @@ -3228,7 +3228,7 @@ bool ProxySQL_Admin::GenericRefreshStatistics(const char *query_no_space, unsign SQLite3_result * ProxySQL_Admin::generate_show_fields_from(const char *tablename, char **err) { char *tn=NULL; // tablename // note that tablename is passed with a trailing ' - tn=(char *)malloc(strlen(tablename)); + tn=(char *)malloc(strlen(tablename) + 1); unsigned int i=0, j=0; while (i Date: Thu, 19 Aug 2021 21:39:42 +0200 Subject: [PATCH 4/8] Fixed memory leak detected by ASAN during 'PROXYSQL SHUTDOWN SLOW' #3554 --- src/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.cpp b/src/main.cpp index 5df0cc98c..c00a189c7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1114,6 +1114,7 @@ void ProxySQL_Main_join_all_threads() { if (GloMyMon && MyMon_thread) { cpu_timer t; MyMon_thread->join(); + delete MyMon_thread; MyMon_thread = NULL; #ifdef DEBUG std::cerr << "GloMyMon joined in "; From fd9f3f13a3d99aa828ed9cd0625d87a9991dd486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 19 Aug 2021 21:40:54 +0200 Subject: [PATCH 5/8] Fixed 'strcat-param-overlap' found by ASAN in 'IsKeepMultiplexEnabledVariables' #3554 --- lib/mysql_connection.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/mysql_connection.cpp b/lib/mysql_connection.cpp index e1d6352af..699cc0d75 100644 --- a/lib/mysql_connection.cpp +++ b/lib/mysql_connection.cpp @@ -2254,7 +2254,11 @@ bool MySQL_Connection::IsKeepMultiplexEnabledVariables(char *query_digest_text) } while (query_digest_text_filter_select && (match = strcasestr(query_digest_text_filter_select,"@@"))) { *match = '\0'; - strcat(query_digest_text_filter_select, match+strlen("@@")); + if (strlen(query_digest_text_filter_select) == 0) { + memcpy(query_digest_text_filter_select, match, strlen("@@")); + } else { + strcat(query_digest_text_filter_select, match+strlen("@@")); + } } std::vectorquery_digest_text_filter_select_v; From d271ef9613fdb0ff41278713cf2d1506170ba43a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 19 Aug 2021 21:47:16 +0200 Subject: [PATCH 6/8] Fixed 'stack-buffer-overflow' found by ASAN during SHA1 generation #3554 --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index c00a189c7..3f95abb57 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1710,7 +1710,7 @@ int main(int argc, const char * argv[]) { SHA1(fb, statbuf.st_size, temp); binary_sha1 = (char *)malloc(SHA_DIGEST_LENGTH*2+1); memset(binary_sha1, 0, SHA_DIGEST_LENGTH*2+1); - char buf[SHA_DIGEST_LENGTH*2]; + char buf[SHA_DIGEST_LENGTH*2 + 1]; for (int i=0; i < SHA_DIGEST_LENGTH; i++) { sprintf((char*)&(buf[i*2]), "%02x", temp[i]); } From 59bd75be4546e1cba08ae87874699047bb6b88a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Thu, 19 Aug 2021 21:49:08 +0200 Subject: [PATCH 7/8] Fixed faulty check in 'test_firewall-t' preventing it from working in DEBUG mode #3554 --- test/tap/tests/test_firewall-t.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/tap/tests/test_firewall-t.cpp b/test/tap/tests/test_firewall-t.cpp index 70fbafbee..39ccf4bd6 100644 --- a/test/tap/tests/test_firewall-t.cpp +++ b/test/tap/tests/test_firewall-t.cpp @@ -59,7 +59,8 @@ int main(int argc, char** argv) { // Test that firewall initialized and blocks all queries if (mysql_query(mysql, "select @@version")) { - ok(mysql_num_rows(result) == 0, "Any query should be blocked"); + int myerrno = mysql_errno(mysql); + ok(myerrno == 1148, "Any query should be blocked"); } // enable 'Select 1' query From bf6e990fb39149fc24da8371f41c43203f025afa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Thu, 19 Aug 2021 22:38:25 +0200 Subject: [PATCH 8/8] Reenable code coverage for TAP tests --- test/tap/tests/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index 8fd2e2de1..99baf3268 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -85,7 +85,7 @@ clean: WITHGCOVVAR := $(shell echo $(WITHGCOV)) ifeq ($(WITHGCOVVAR),1) -WGCOV=-DWITHGCOV --coverage +WGCOV=-DWITHGCOV --coverage -lgcov else WGCOV= endif @@ -97,7 +97,7 @@ else WASAN= endif -OPT=-O2 -Wl,--no-as-needed +OPT=-O2 $(WGCOV) -Wl,--no-as-needed debug: OPT=-O0 -DDEBUG -ggdb -Wl,--no-as-needed $(WGCOV) $(WASAN) debug: tests