From: Staale Smedseng Date: March 17 2009 4:03pm Subject: bzr commit into mysql-5.1-bugteam branch (staale.smedseng:2839) Bug#42502 List-Archive: http://lists.mysql.com/commits/69468 X-Bug: 42502 Message-Id: <20090317160402.C6011D6A2B8@atum21.norway.sun.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3616170723915397034==" --===============3616170723915397034== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///export/home/tmp/ss156133/z/b42502-51/ based on revid:azundris@stripped 2839 Staale Smedseng 2009-03-17 Bug #42502 huge memory leak possible with timezone functions Unknown timezone specifications are properly rejected by the server, but are copied into tz_storage before rejection, and hence is retained until end of server life. With sufficiently large bogus timezone specs, it is easy to exhaust system memory. Allocation of memory for a copy of the timezone name is delayed until after verification of validity, at the cost of a memcpy of the timezone info (currently 88 bytes). modified: sql/tztime.cc === modified file 'sql/tztime.cc' --- a/sql/tztime.cc 2009-01-15 18:11:25 +0000 +++ b/sql/tztime.cc 2009-03-17 16:03:53 +0000 @@ -1825,23 +1825,14 @@ tz_load_from_open_tables(const String *t #ifdef ABBR_ARE_USED char chars[max(TZ_MAX_CHARS + 1, (2 * (MY_TZNAME_MAX + 1)))]; #endif - DBUG_ENTER("tz_load_from_open_tables"); - - /* Prepare tz_info for loading also let us make copy of time zone name */ - if (!(alloc_buff= (char*) alloc_root(&tz_storage, sizeof(TIME_ZONE_INFO) + - tz_name->length() + 1))) - { - sql_print_error("Out of memory while loading time zone description"); - return 0; - } - tz_info= (TIME_ZONE_INFO *)alloc_buff; - bzero(tz_info, sizeof(TIME_ZONE_INFO)); - tz_name_buff= alloc_buff + sizeof(TIME_ZONE_INFO); - /* - By writing zero to the end we guarantee that we can call ptr() - instead of c_ptr() for time zone name. + /* + Used as a temporary tz_info until we decide that we actually want to + allocate and keep the tz info and tz name in tz_storage. */ - strmake(tz_name_buff, tz_name->ptr(), tz_name->length()); + TIME_ZONE_INFO tmp_tz_info; + bzero(&tmp_tz_info, sizeof(TIME_ZONE_INFO)); + + DBUG_ENTER("tz_load_from_open_tables"); /* Let us find out time zone id by its name (there is only one index @@ -1866,7 +1857,8 @@ tz_load_from_open_tables(const String *t Most probably user has mistyped time zone name, so no need to bark here unless we need it for debugging. */ - sql_print_error("Can't find description of time zone '%s'", tz_name_buff); + String tmp(*tz_name); + sql_print_error("Can't find description of time zone '%s'", tmp.c_ptr()); #endif goto end; } @@ -1895,8 +1887,8 @@ tz_load_from_open_tables(const String *t /* If Uses_leap_seconds == 'Y' */ if (table->field[1]->val_int() == 1) { - tz_info->leapcnt= tz_leapcnt; - tz_info->lsis= tz_lsis; + tmp_tz_info.leapcnt= tz_leapcnt; + tmp_tz_info.lsis= tz_lsis; } (void)table->file->ha_index_end(); @@ -1932,18 +1924,18 @@ tz_load_from_open_tables(const String *t #ifdef ABBR_ARE_USED // FIXME should we do something with duplicates here ? table->field[4]->val_str(&abbr, &abbr); - if (tz_info->charcnt + abbr.length() + 1 > sizeof(chars)) + if (tmp_tz_info.charcnt + abbr.length() + 1 > sizeof(chars)) { sql_print_error("Error while loading time zone description from " "mysql.time_zone_transition_type table: not enough " "room for abbreviations"); goto end; } - ttis[ttid].tt_abbrind= tz_info->charcnt; - memcpy(chars + tz_info->charcnt, abbr.ptr(), abbr.length()); - tz_info->charcnt+= abbr.length(); - chars[tz_info->charcnt]= 0; - tz_info->charcnt++; + ttis[ttid].tt_abbrind= tmp_tz_info.charcnt; + memcpy(chars + tmp_tz_info.charcnt, abbr.ptr(), abbr.length()); + tmp_tz_info.charcnt+= abbr.length(); + chars[tmp_tz_info.charcnt]= 0; + tmp_tz_info.charcnt++; DBUG_PRINT("info", ("time_zone_transition_type table: tz_id=%u tt_id=%u tt_gmtoff=%ld " @@ -1956,9 +1948,9 @@ tz_load_from_open_tables(const String *t #endif /* ttid is increasing because we are reading using index */ - DBUG_ASSERT(ttid >= tz_info->typecnt); + DBUG_ASSERT(ttid >= tmp_tz_info.typecnt); - tz_info->typecnt= ttid + 1; + tmp_tz_info.typecnt= ttid + 1; res= table->file->index_next_same(table->record[0], table->field[0]->ptr, 4); @@ -1990,14 +1982,14 @@ tz_load_from_open_tables(const String *t ttime= (my_time_t)table->field[1]->val_int(); ttid= (uint)table->field[2]->val_int(); - if (tz_info->timecnt + 1 > TZ_MAX_TIMES) + if (tmp_tz_info.timecnt + 1 > TZ_MAX_TIMES) { sql_print_error("Error while loading time zone description from " "mysql.time_zone_transition table: " "too much transitions"); goto end; } - if (ttid + 1 > tz_info->typecnt) + if (ttid + 1 > tmp_tz_info.typecnt) { sql_print_error("Error while loading time zone description from " "mysql.time_zone_transition table: " @@ -2005,9 +1997,9 @@ tz_load_from_open_tables(const String *t goto end; } - ats[tz_info->timecnt]= ttime; - types[tz_info->timecnt]= ttid; - tz_info->timecnt++; + ats[tmp_tz_info.timecnt]= ttime; + types[tmp_tz_info.timecnt]= ttid; + tmp_tz_info.timecnt++; DBUG_PRINT("info", ("time_zone_transition table: tz_id: %u tt_time: %lu tt_id: %u", @@ -2032,6 +2024,34 @@ tz_load_from_open_tables(const String *t table= 0; /* + Let us check how correct our time zone description is. We don't check for + tz->timecnt < 1 since it is ok for GMT. + */ + if (tmp_tz_info.typecnt < 1) + { + sql_print_error("loading time zone without transition types"); + goto end; + } + + /* Allocate memory for the timezone info and timezone name in tz_storage. */ + if (!(alloc_buff= (char*) alloc_root(&tz_storage, sizeof(TIME_ZONE_INFO) + + tz_name->length() + 1))) + { + sql_print_error("Out of memory while loading time zone description"); + return 0; + } + + /* Move the temporary tz_info into the allocated area */ + tz_info= (TIME_ZONE_INFO *)alloc_buff; + memcpy(tz_info, &tmp_tz_info, sizeof(TIME_ZONE_INFO)); + tz_name_buff= alloc_buff + sizeof(TIME_ZONE_INFO); + /* + By writing zero to the end we guarantee that we can call ptr() + instead of c_ptr() for time zone name. + */ + strmake(tz_name_buff, tz_name->ptr(), tz_name->length()); + + /* Now we will allocate memory and init TIME_ZONE_INFO structure. */ if (!(alloc_buff= (char*) alloc_root(&tz_storage, @@ -2062,15 +2082,7 @@ tz_load_from_open_tables(const String *t tz_info->ttis= (TRAN_TYPE_INFO *)alloc_buff; memcpy(tz_info->ttis, ttis, tz_info->typecnt * sizeof(TRAN_TYPE_INFO)); - /* - Let us check how correct our time zone description and build - reversed map. We don't check for tz->timecnt < 1 since it ok for GMT. - */ - if (tz_info->typecnt < 1) - { - sql_print_error("loading time zone without transition types"); - goto end; - } + /* Build reversed map. */ if (prepare_tz_info(tz_info, &tz_storage)) { sql_print_error("Unable to build mktime map for time zone"); --===============3616170723915397034== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/staale.smedseng@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: staale.smedseng@stripped # target_branch: file:///export/home/tmp/ss156133/z/b42502-51/ # testament_sha1: 3f17b8e80c14b54148e34bfb938795a6d6fc4129 # timestamp: 2009-03-17 17:04:02 +0100 # base_revision_id: azundris@stripped # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWYjIfigAA15fgEAQe////3+n ntq////6YAhO21d3d27nxlbd6ynIGqW0vNW7uZYSSI0CaaGqeT1U/0aCMSnsp6kep5I9TMhppoj9 U3qg9QSSFM1TyehpMQKGm1MjRkaBo0GgAAAGmo1MTSR6RphMhoDRoDI0yaNNAeoAyASIhCaKn+kG TJkaaaSPKA0AaAABoGQ4BhGE0xDAIBkAMI0yZMIwENBJEIyBDQqfjUyaCniTMppkM0DUNAAHpML0 hZj0La+MNU0hhZMNiyR/bz8jVDLquxS57xBUuXoMXMpxj08NS6Ka4ufTRjY8Rnq1Kw1ZvotrpTj+ MLSwC7B7quJt7KnF3FfS0YVHvKnJC1vs0wEhZwSYZCPNzRR+e8zTvDuyzMx5WcmsZnSc8BbT62sz CX9VNrAyzWjkvOPbQ3bo4tuH2Ob4ODL8ThIN0u7prc7We9oiadopKSCx6TS12TVmI7ba9H4NNtfc +JtJglqIiO2q9vrffM8P+go8zI9iM54Rh6STKO8rhOzImgevNN1Mw8iswYyeWmoP3M0PjGWLGKGi mtZ08NKD5XUiC6zVSvqqJ/i1+iocYQax+cn+nO42aOapi7C/NROe29uOd03QVcpUFcZQOlkIDpcO GZ5jg+gapyJw7eeG/brn3bGe78Wut9XbzIr492kohtLmLd8UbwjAfUtuyOA22x91dwfDG+WBKG61 yiadta7tqOI1HOkVCQqo9twRVxheCVJ0SBQEx0jVXsc8WSamMvwrk0FgjRp3TrdLtWvgLR0xjz1S 520tk3DRozQWItMwYTtKZoJOD7sHVFtA76O1G6NlnU7wUs4tdQ5bPF2VWSNI89Nx2ClBSoqqMIPA 3llXFPrLCLy2YeOcoYmnMe/UCjzvB3EwiLFkN8jCNVVpztSvPo95eVSwFWWIPREhagyrdIdoTMDf cv7HVSnnxY1R9rMCZGhqjYzjSLScS8hOoKCuOpaZ22QsSgaWlFNi9EFw1sqQFHXOqUyqgyJmrhJ1 WkdVikZujKUhW7zMcGnI5hfvaNpmZbHSNRNZyz03GjKC3hfzKxQJCfIvIbU8gbK4kasm1khYHAut tJ/O5xmZVX3vnLleevfVWbFuvUS5WDCwjfA9D1TTJXlBrOQyMRTtqaRouB4xtpcaRpXMQJs/QzS6 FjSrtuiK1ENbNF9W2pdRyMvGzotJMitAxoWHjVYE53sMoWFYS8y7fO9Hc7GJs+OEMsDooQMpnyFD C9zE8/spdRCeMa/1VPG23pXucvWw+79DhFqsDtNGFq74GORCifnu+rAkEHzo1JLtIvxy/AQhCFD7 9yVIpW4pcYeAni4i/wV/AX+MsVHLBT4wYG/OuR5SoWjaHDmWrOLR/rij38jDOuVnHPBJDgwHepEb X76+hqw60gpirKw9hl7qAUrDVY9nrTdx8j3HtCHAKxg9oqZvLvA9/jZP3bHdDfC4Fek/uqAZnfFq sBgjoYVa2i5YKZGufZIcuFmhPyceJ4Hp3rQj4ETNuxfpCxuhEMWGANFGPAT5QPYdKr+DuMF5BXYz Mtw9Xo4XiZ14l1manA97BtrVDiC9IsjeuSoGfDGcyZQY3SN7JHPPr9k4i+s3FyqnmYwLz7TIhliO R++NSvA3YKGpjP09Oj3kKovUf9CuOwkNykaUkUm43pv+iATlYj4tRkzNtdSApUusuhUZHmHNxzKR 8SpcC63zuYGNpfEzEg0xxit20zcJFB0zMNP5FDn3cr+jGgFWzKxtbmHMVVgMZtCuAxRMw3p0bEbJ kaNdKHGyWuR5VCARGqtHmtuZeNeCRnopAURlgGdhD5LSR38TkKlImxuIHYiI2lyH6dWddu7lSE9F TLgU75dd+VHMyUHPcgfTv/16KObHnOzKlZtGuBkiB9jINMCzUeW0oVy1CbE1iUXBtd+odscpB0Ca rZc55esGaPWUQzSLLTsZPSROqn9ITYcUFgSReSpmdSO7nnC6tJkmKlBEB1DgBKXbkVV9dzi19BIO By7edjU0RPfbqDPv2l508B2fuRoXMOuS7TXgxuck5RX/YdYxHa2EYXuZtTpevEBWUT9UVmgzPxLK kTi1KzrnQ0MGDR9mxK3Ji/PlZB2+bP5Z0ZGhkg6CBm3+IWwrojqaIxBskmSznDcDpjounObwSxwD cq8F8icglRg0qtHufly6fj04F+HYEMW5NKVjTQoJGm0NYa2mNmDiGkHltg7REezWgojc94K1KxgO 1CYmlEsQ6uA7iAQc+SGHPmX18PrbkuqwbEHjgRSbt3AoxKCPODSQ45XQLq9j+unykNknwOPlqxV1 nkuAtwE4WyOLGDaHLWxFJ3gztHUCa1kYm1v5+jwpRonxeysbGn10NFq65rFb67O44CNM1rw4sxmk Z0YKNNCtTJ7AwQMMKzFewHb1WjTVRiOFSJl7SV5q84HfrKwaSuNAMjCUUWrmS713HI0jydAcV1Cz m02f5YWJpZqVBWFJvSda+A5ijkYzI+10IRXnmCIT2RuscRjLUwc5zVJRbIVJ5aIShF9xurxeSmWo ekO6J0o3ytWR5KnD3WqIMPDS3EcxN2BY+I+fiSeuCXBEksydm9QDWKJ66UWPNCyyxvbzvPl14XO8 LWY2X8EgcuXJxExiY6NhpyW9PBGT4dbanpNnV5Punsioy0fzJF4QfcrR+bn73Ae3IS/r1cymAibs JQnoTNXsH4scY9rIy0jeqBdOQX4TRYDSAtyS0W8btBRNNjfXer1AP+LuSKcKEhEZD8UA --===============3616170723915397034==--