- developers-list - monetdb.org

Re: MonetDB: Jul2015 - Try not to overflow snprintf buffers.
by Ying Zhang 24 Jul '15

24 Jul '15

> On Jul 24, 2015, at 16:36, Sjoerd Mullender <commits(a)monetdb.org> wrote: > > Changeset: c84b68960bca for MonetDB > URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=c84b68960bca > Modified Files: > clients/mapiclient/ReadlineTools.c > clients/mapiclient/dump.c > clients/mapilib/mapi.c > clients/odbc/driver/SQLExecute.c > common/options/monet_options.c > common/stream/stream.c > common/utils/mutils.c > gdk/gdk_bbp.c > Branch: Jul2015 > Log Message: > > Try not to overflow snprintf buffers. > > > diffs (truncated from 373 to 300 lines): > > diff --git a/clients/mapiclient/ReadlineTools.c b/clients/mapiclient/ReadlineTools.c > --- a/clients/mapiclient/ReadlineTools.c > +++ b/clients/mapiclient/ReadlineTools.c > @@ -61,12 +61,16 @@ sql_tablename_generator(const char *text > static MapiHdl table_hdl; > > if (!state) { > - char query[512]; > + char *query; > > seekpos = 0; > len = strlen(text); > - snprintf(query, sizeof(query), "SELECT t.\"name\", s.\"name\" FROM \"sys\".\"tables\" t, \"sys\".\"schemas\" s where t.schema_id = s.id AND t.\"name\" like '%s%%'", text); > - if ((table_hdl = mapi_query(_mid, query)) == NULL || mapi_error(_mid)) { > + if ((query = malloc(len + 128)) == NULL) > + return NULL; > + snprintf(query, len + 128, "SELECT t.\"name\", s.\"name\" FROM \"sys\".\"tables\" t, \"sys\".\"schemas\" s where t.schema_id = s.id AND t.\"name\" like '%s%%'", text); > + table_hdl = mapi_query(_mid, query); > + free(query); > + if (table_hdl == NULL || mapi_error(_mid)) { > if (table_hdl) { > mapi_explain_query(table_hdl, stderr); > mapi_close_handle(table_hdl); > @@ -176,7 +180,7 @@ static char *mal_commands[] = { > static int > mal_help(int cnt, int key) > { > - char *name, *c, buf[512]; > + char *name, *c, *buf; > int seekpos = 0, rowcount; > MapiHdl table_hdl; > > @@ -188,8 +192,12 @@ mal_help(int cnt, int key) > c--; > while (c > rl_line_buffer && !isspace(*c)) > c--; > - snprintf(buf, sizeof(buf), "manual.help(\"%s\");", c); > - if ((table_hdl = mapi_query(_mid, buf)) == NULL || mapi_error(_mid)) { > + if ((buf = malloc(strlen(c) + 20)) == NULL) > + return 0; > + snprintf(buf, strlen(c) + 20, "manual.help(\"%s\");", c); > + table_hdl = mapi_query(_mid, buf); > + free(buf); > + if (table_hdl == NULL || mapi_error(_mid)) { > if (table_hdl) { > mapi_explain_query(table_hdl, stderr); > mapi_close_handle(table_hdl); > @@ -220,7 +228,7 @@ mal_command_generator(const char *text, > static int idx; > static int seekpos, len, rowcount; > static MapiHdl table_hdl; > - char *name, buf[512]; > + char *name, *buf; > > /* we pick our own portion of the linebuffer */ > text = rl_line_buffer + strlen(rl_line_buffer) - 1; > @@ -250,14 +258,18 @@ mal_command_generator(const char *text, > text = c + 2; > while (isspace((int) *text)) > text++; > + if ((buf = malloc(strlen(text) + 32)) == NULL) > + return NULL; > if (strchr(text, '.') == NULL) > - snprintf(buf, sizeof(buf), > + snprintf(buf, strlen(text) + 32, > "manual.completion(\"%s.*(\");", text); > else > - snprintf(buf, sizeof(buf), > + snprintf(buf, strlen(text) + 32, > "manual.completion(\"%s(\");", text); > seekpos = 0; > - if ((table_hdl = mapi_query(_mid, buf)) == NULL || mapi_error(_mid)) { > + table_hdl = mapi_query(_mid, buf); > + free(buf); > + if (table_hdl == NULL || mapi_error(_mid)) { > if (table_hdl) { > mapi_explain_query(table_hdl, stderr); > mapi_close_handle(table_hdl); > diff --git a/clients/mapiclient/dump.c b/clients/mapiclient/dump.c > --- a/clients/mapiclient/dump.c > +++ b/clients/mapiclient/dump.c > @@ -212,7 +212,7 @@ dump_foreign_keys(Mapi mid, const char * > "fkt.name = '%s' " > "ORDER BY fkk.name, nr", schema, tname); > } else if (tid != NULL) { > - maxquerylen = 1024; > + maxquerylen = 1024 + strlen(tid); > query = malloc(maxquerylen); > snprintf(query, maxquerylen, > "SELECT ps.name, " /* 0 */ > @@ -529,6 +529,8 @@ dump_column_definition(Mapi mid, stream > maxquerylen = 1024; > if (tid == NULL) > maxquerylen += strlen(tname) + strlen(schema); > + else > + maxquerylen += strlen(tid); > if ((query = malloc(maxquerylen)) == NULL) > goto bailout; > > diff --git a/clients/mapilib/mapi.c b/clients/mapilib/mapi.c > --- a/clients/mapilib/mapi.c > +++ b/clients/mapilib/mapi.c > @@ -1918,7 +1918,7 @@ mapi_new(void) > Mapi > mapi_mapiuri(const char *url, const char *user, const char *pass, const char *lang) > { > - char uri[8096]; > + char *uri; > char *host; > int port; > char *dbname; > @@ -1961,12 +1961,13 @@ mapi_mapiuri(const char *url, const char > } > > /* copy to a writable working buffer */ > - snprintf(uri, 8096, "%s", url + sizeof("mapi:monetdb://") - 1); > + uri = strdup(url + sizeof("mapi:monetdb://") - 1); > > if (uri[0] != '/') { > if ((p = strchr(uri, ':')) == NULL) { > mapi_setError(mid, "URI must contain a port number after " > "the hostname", "mapi_mapiuri", MERROR); > + free(uri); > return mid; > } > *p++ = '\0'; > @@ -1990,6 +1991,7 @@ mapi_mapiuri(const char *url, const char > if (port <= 0) { > mapi_setError(mid, "URI contains invalid port", > "mapi_mapiuri", MERROR); > + free(uri); > return mid; > } > } else { > @@ -2029,6 +2031,7 @@ mapi_mapiuri(const char *url, const char > mid->database = strdup(dbname); > > set_uri(mid); > + free(uri); > > return mid; > } > @@ -2128,7 +2131,8 @@ mapi_mapi(const char *host, int port, co > while ((e = readdir(d)) != NULL) { > if (strncmp(e->d_name, ".s.monetdb.", 11) != 0) > continue; > - snprintf(buf, sizeof(buf), "/tmp/%s", e->d_name); > + if (snprintf(buf, sizeof(buf), "/tmp/%s", e->d_name) >= sizeof(buf)) Hai Sjoerd, This line triggers the compiler error: /.../src/MonetDB-11.21/clients/mapilib/mapi.c:2134:59: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] if (snprintf(buf, sizeof(buf), "/tmp/%s", e->d_name) >= sizeof(buf)) Jennie > + continue; /* ignore long name */ > if (stat(buf, &st) != -1 && S_ISSOCK(st.st_mode)) > socks[i++] = atoi(e->d_name + 11); > if (i == sizeof(socks)) > @@ -2320,7 +2324,8 @@ parse_uri_query(Mapi mid, char *uri) > static void > set_uri(Mapi mid) > { > - char uri[1024]; > + size_t urilen = strlen(mid->hostname) + (mid->database ? strlen(mid->database) : 0) + 32; > + char *uri = malloc(urilen); > > /* uri looks as follows: > * mapi:monetdb://host:port/database > @@ -2330,25 +2335,25 @@ set_uri(Mapi mid) > > if (mid->database != NULL) { > if (mid->hostname[0] == '/') { > - snprintf(uri, sizeof(uri), "mapi:monetdb://%s?database=%s", > - mid->hostname, mid->database); > + snprintf(uri, urilen, "mapi:monetdb://%s?database=%s", > + mid->hostname, mid->database); > } else { > - snprintf(uri, sizeof(uri), "mapi:monetdb://%s:%d/%s", > - mid->hostname, mid->port, mid->database); > + snprintf(uri, urilen, "mapi:monetdb://%s:%d/%s", > + mid->hostname, mid->port, mid->database); > } > } else { > if (mid->hostname[0] == '/') { > - snprintf(uri, sizeof(uri), "mapi:monetdb://%s", > - mid->hostname); > + snprintf(uri, urilen, "mapi:monetdb://%s", > + mid->hostname); > } else { > - snprintf(uri, sizeof(uri), "mapi:monetdb://%s:%d", > - mid->hostname, mid->port); > + snprintf(uri, urilen, "mapi:monetdb://%s:%d", > + mid->hostname, mid->port); > } > } > > if (mid->uri != NULL) > free(mid->uri); > - mid->uri = strdup(uri); > + mid->uri = uri; > } > > /* (Re-)establish a connection with the server. */ > @@ -2657,7 +2662,7 @@ mapi_start_talking(Mapi mid) > pwdhash = mcrypt_MD5Sum(mid->password, > strlen(mid->password)); > } else { > - snprintf(buf, BLOCK, "server requires unknown hash '%s'", > + snprintf(buf, BLOCK, "server requires unknown hash '%.100s'", > serverhash); > close_connection(mid); > return mapi_setError(mid, buf, "mapi_start_talking", MERROR); > @@ -2688,7 +2693,7 @@ mapi_start_talking(Mapi mid) > } > if (hash == NULL) { > /* the server doesn't support what we can */ > - snprintf(buf, BLOCK, "unsupported hash algorithms: %s", hashes); > + snprintf(buf, BLOCK, "unsupported hash algorithms: %.100s", hashes); > close_connection(mid); > return mapi_setError(mid, buf, "mapi_start_talking", MERROR); > } > @@ -2697,14 +2702,18 @@ mapi_start_talking(Mapi mid) > > /* note: if we make the database field an empty string, it > * means we want the default. However, it *should* be there. */ > - snprintf(buf, BLOCK, "%s:%s:%s:%s:%s:\n", > + if (snprintf(buf, BLOCK, "%s:%s:%s:%s:%s:\n", > #ifdef WORDS_BIGENDIAN > - "BIG", > + "BIG", > #else > - "LIT", > + "LIT", > #endif > - mid->username, hash, mid->language, > - mid->database == NULL ? "" : mid->database); > + mid->username, hash, mid->language, > + mid->database == NULL ? "" : mid->database) >= BLOCK) {; > + mapi_setError(mid, "combination of database name and user name too long", "mapi_start_talking", MERROR); > + free(hash); > + return mid->error; > + } > > free(hash); > } else { > @@ -2873,7 +2882,7 @@ mapi_start_talking(Mapi mid) > } else { > char re[BUFSIZ]; > snprintf(re, sizeof(re), > - "error while parsing redirect: %s\n", red); > + "error while parsing redirect: %.100s\n", red); > mapi_close_handle(hdl); > mapi_setError(mid, re, "mapi_start_talking", MERROR); > return mid->error; > diff --git a/clients/odbc/driver/SQLExecute.c b/clients/odbc/driver/SQLExecute.c > --- a/clients/odbc/driver/SQLExecute.c > +++ b/clients/odbc/driver/SQLExecute.c > @@ -443,8 +443,7 @@ MNDBExecute(ODBCStmt *stmt) > addStmtError(stmt, "HY001", NULL, 0); > return SQL_ERROR; > } > - snprintf(query, querylen, "execute %d (", stmt->queryid); > - querypos = strlen(query); > + querypos = snprintf(query, querylen, "execute %d (", stmt->queryid); > /* XXX fill in parameter values */ > if (desc->sql_desc_bind_offset_ptr) > offset = *desc->sql_desc_bind_offset_ptr; > diff --git a/common/options/monet_options.c b/common/options/monet_options.c > --- a/common/options/monet_options.c > +++ b/common/options/monet_options.c > @@ -216,7 +216,6 @@ mo_builtin_settings(opt **Set) > { > int i = 0; > opt *set; > - char buf[BUFSIZ]; > > if (Set == NULL) > return 0; > @@ -228,10 +227,8 @@ mo_builtin_settings(opt **Set) > > set[i].kind = opt_builtin; > set[i].name = strdup("gdk_dbpath"); > - snprintf(buf, BUFSIZ, "%s%c%s%c%s%c%s", > - LOCALSTATEDIR, DIR_SEP, "monetdb5", DIR_SEP, "dbfarm", > - DIR_SEP, "demo"); > - set[i].value = strdup(buf); > + set[i].value = strdup(LOCALSTATEDIR DIR_SEP_STR "monetdb5" DIR_SEP_STR > + "dbfarm" DIR_SEP_STR "demo"); > i++; > set[i].kind = opt_builtin; > set[i].name = strdup("gdk_debug"); > diff --git a/common/stream/stream.c b/common/stream/stream.c > --- a/common/stream/stream.c > +++ b/common/stream/stream.c > @@ -510,16 +510,19 @@ error(stream *s) > > switch (s->errnr) { > case MNSTR_OPEN_ERROR: > - snprintf(buf, sizeof(buf), "error could not open file %s\n", s->name); > + snprintf(buf, sizeof(buf), "error could not open file %.100s\n", > + s->name); > return strdup(buf); > case MNSTR_READ_ERROR: > - snprintf(buf, sizeof(buf), "error reading file %s\n", s->name); > + snprintf(buf, sizeof(buf), "error reading file %.100s\n", > + s->name); > _______________________________________________ > checkin-list mailing list > checkin-list(a)monetdb.org > https://www.monetdb.org/mailman/listinfo/checkin-list

1 0

MonetDB causing disk space to fill up
by Anu Raj Srivastava 19 Jul '15

19 Jul '15

Hi, I have a bunch of test queries which I am testing against MonetDB. Queries all by itself run fine but if you run the same set of queries in a loop disk runs out of space. I am not sure why would memory map files won't clean up automatically and why do we have to start the server again to clean up these files? Is there a workaround to this problem or is this a known issue in MonetDB? Thanks, Anu

3 7

Re: Re: Re: mserver5 stability problems in acting with multithreaded clients
by Sherzod Mutalov 18 Jul '15

18 Jul '15

To: sjoerd(a)monetdb.org I build Jul2015 branch and tested with my test app and mserver5 errors disappeared :) Nice But, still have problem with Mapi library, where first 1-4 connections get lost, because WSAStartup error. Then I added changes to my test program, and when program starts it preconfigures Windows Sockets, then starts client thread. Now it works: https://gist.github.com/shmutalov/3894fdd22ec62334fe17 Regards, Sherzod Mutalov

2 1

Re: Re: mserver5 stability problems in acting with multithreaded clients
by Sherzod Mutalov 17 Jul '15

17 Jul '15

Thank you for answer, <b>Sjoerd Mullender! You said, > </b>In all, I think the situation is better in Jul2015 Does these problems have been fixed in nightly build? Regards, Sherzod Mutalov

2 1

Re: Re: mserver5 stability problems in acting with multithreaded clients
by Sherzod Mutalov 17 Jul '15

17 Jul '15

You can get MapiDemoCpp source and test it:     https://gist.github.com/shmutalov/3894fdd22ec62334fe17 Regards, Sherzod Mutalov

2 1

mserver5 stability problems in acting with multithreaded clients
by Sherzod Mutalov 17 Jul '15

17 Jul '15

Good day all, I am testing mserver5 with multithreaded client and have stability problems. I wrote a simple app in C#, which uses wrapper around Mapi (pure calls to libmapi.dll functions). The app creates 1000 tasks (threads), each thread opens connection to mserver5, then sends a simple query: "SELECT {0}, NOW();", where "{0}" is thread number, and fetchs the result. Problems are: 1. When my application starts to execute threads, always 1st, 2nd or 3rd connection will be lost, and I can't reconnect from that thread. 2. Sometimes there is error on mserver5, which causes connection lost on the client:     "!SABAOTHgetMyStatus: MALException:sabaoth.getmystatus:could not find my own database?!?" mserver5 is running on the local machine, same as client. I tryied to set a gdk_nr_threads option to 1, 8, 16, and get the same problems. I am running on Windows 7 x64, 16GB RAM, Intel i7-5500 CPU Regards, Sherzod Mutalov

1 0

Job openings at MonetDB Solutions
by Ying Zhang 14 Jul '15

14 Jul '15

Dear all, MonetDB Solutions, the supporting company of MonetDB, is looking for core database developers. If you’re interested, please see the job description here: https://www.monetdbsolutions.com/company/careers Regards, Jennie

1 0

Re: Re: mserver5 crash while login with newly created user
by Sherzod Mutalov 14 Jul '15

14 Jul '15

> > > Adding users on the mal level isn't supported on the sql level. > > Ok, how can I create sql user on server side? Is there a way to obtain this?

2 1

mserver5 crash while login with newly created user
by Sherzod Mutalov 14 Jul '15

14 Jul '15

I want to change default monetdb user password to other and create new database user when starting new database instance, before any client connects to newly created db. I starting server with follow command: mserver5.exe --dbinit="oldpwd := clients.backendsum(\"monetdb\"); newpwd := clients.backendsum(\"secret\"); clients.changePassword(oldpwd, newpwd); usrpwd:=clients.backendsum(\"secret2\");clients.addUser(\"newuser\", usrpwd);" --dbpath="D:\Projects\git\monetdb-latest-dev\NT\build\bin\dbfarm\demo" There are no errors. Then I trying to connect to this server: mclient.exe -u monetdb -P secret No errors, connected. Then, I am trying to open new session: mclient.exe -u newuser -P secret2 Ooops! Client exits with error: Connection terminated Server unexpectedly dies. I restarted server, then try connect one more time, same behaviour: "Connection terminated". Ok. Then I connected to server with default "monetdb" user and checked "users" table: mclient.exe -u monetdb -P secret sql>select * from users; +---------+---------------+----------------+ | name | fullname | default_schema | +=========+===============+================+ | monetdb | MonetDB Admin | 2000 | | newuser | null | null | +---------+---------------+----------------+ 2 tuples (5.332ms) Look, fullname and default_schema for "newuser" is null. I think "null pointer dereference" somewhere in code. If I creating user with SQL (CREATE USER ...) everything works fine, I can connect.

2 1

mserver5 crash while login with newly created user
by Sherzod Mutalov 14 Jul '15

14 Jul '15

I tested with monetdb v11.19.13 and v11.19.15 (Windows 7 x64, MSVC2013)

1 0