Skip to content

Commit e2a755f

Browse files
Shuxin Yangautarch
authored andcommitted
Make sure to set pointers to NULL after freeing them
The dangling pointer could incur real problem. This is the problem I run into: My system is runing nginx built along with ngx_http_geoip2_module and libmaxminddb. nginx is running with a configuration with a geoip2-directive specifying a file that dose not exist. like this: ----------------------------------------------------------- geoip2 /my/path/that/dose/not/exist/maxmind-country.mmdb { $geoip2_data_country_code default=US country iso_code; $geoip2_data_country_name country names en; } ----------------------------------------------------------- When Nginx is launched with this configuration, ngx_http_geoip2_module call MMDB_open() which in turn calls free_mmdb_struct() as MMDB_open() was not successfully open the specified .mmdb file. Nginx then exit, calling ngx_http_geoip2_cleanup() which call MMDB_close(), which call free_mmdb_struct(). NOTE that in this senario, free_mmdb_struct() is called *TWICE* upon the same mmdb! The first call to free_mmdb_struct() yields bunch of dangling pointers, and the 2nd call to free_mmdb_struct() will call free() on these dangling pointers. This fix is just to set those pointer NULL right after the object they pointing to are deallocated.
1 parent 4da7859 commit e2a755f

File tree

2 files changed

+19
-12
lines changed

2 files changed

+19
-12
lines changed

Changes.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
* Fixed a number of small issues found by Coverity.
22

3+
* When freeing the MMDB struct in `MMDB_close()` we make sure to set the
4+
pointers to NULL after freeing the memory they point to. This makes it safe
5+
to call `MMDB_close` more than once on the same `MMDB_s` struct
6+
pointer. Before this change, calling this function twice on the same pointer
7+
could cause the code to free memory that belonged to something else in the
8+
process. Patch by Shuxin Yang. GitHub PR #41.
9+
310
## 1.0.1 - 2014-09-03
411

512
* Added missing LICENSE and NOTICE files to distribution. No code changes.

src/maxminddb.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ LOCAL char *bytes_to_hex(uint8_t *bytes, uint32_t size);
187187
} \
188188
} while (0)
189189

190+
#define FREE_AND_SET_NULL(p) { free((void *)(p)); (p) = NULL; }
191+
190192
int MMDB_open(const char *const filename, uint32_t flags, MMDB_s *const mmdb)
191193
{
192194
mmdb->file_content = NULL;
@@ -1499,7 +1501,7 @@ LOCAL void free_mmdb_struct(MMDB_s *const mmdb)
14991501
}
15001502

15011503
if (NULL != mmdb->filename) {
1502-
free((void *)mmdb->filename);
1504+
FREE_AND_SET_NULL(mmdb->filename);
15031505
}
15041506
if (NULL != mmdb->file_content) {
15051507
#ifdef _WIN32
@@ -1513,7 +1515,7 @@ LOCAL void free_mmdb_struct(MMDB_s *const mmdb)
15131515
}
15141516

15151517
if (NULL != mmdb->metadata.database_type) {
1516-
free((void *)mmdb->metadata.database_type);
1518+
FREE_AND_SET_NULL(mmdb->metadata.database_type);
15171519
}
15181520

15191521
free_languages_metadata(mmdb);
@@ -1527,9 +1529,9 @@ LOCAL void free_languages_metadata(MMDB_s *mmdb)
15271529
}
15281530

15291531
for (size_t i = 0; i < mmdb->metadata.languages.count; i++) {
1530-
free((char *)mmdb->metadata.languages.names[i]);
1532+
FREE_AND_SET_NULL(mmdb->metadata.languages.names[i]);
15311533
}
1532-
free(mmdb->metadata.languages.names);
1534+
FREE_AND_SET_NULL(mmdb->metadata.languages.names);
15331535
}
15341536

15351537
LOCAL void free_descriptions_metadata(MMDB_s *mmdb)
@@ -1542,22 +1544,20 @@ LOCAL void free_descriptions_metadata(MMDB_s *mmdb)
15421544
if (NULL != mmdb->metadata.description.descriptions[i]) {
15431545
if (NULL !=
15441546
mmdb->metadata.description.descriptions[i]->language) {
1545-
free(
1546-
(char *)mmdb->metadata.description.descriptions[i]->
1547-
language);
1547+
FREE_AND_SET_NULL(
1548+
mmdb->metadata.description.descriptions[i]->language);
15481549
}
15491550

15501551
if (NULL !=
15511552
mmdb->metadata.description.descriptions[i]->description) {
1552-
free(
1553-
(char *)mmdb->metadata.description.descriptions[i]->
1554-
description);
1553+
FREE_AND_SET_NULL(
1554+
mmdb->metadata.description.descriptions[i]->description);
15551555
}
1556-
free(mmdb->metadata.description.descriptions[i]);
1556+
FREE_AND_SET_NULL(mmdb->metadata.description.descriptions[i]);
15571557
}
15581558
}
15591559

1560-
free(mmdb->metadata.description.descriptions);
1560+
FREE_AND_SET_NULL(mmdb->metadata.description.descriptions);
15611561
}
15621562

15631563
const char *MMDB_lib_version(void)

0 commit comments

Comments
 (0)