diff options
author | Peter Wu <peter@lekensteyn.nl> | 2018-03-09 15:17:02 +0100 |
---|---|---|
committer | Gerald Combs <gerald@wireshark.org> | 2018-03-09 16:10:32 +0000 |
commit | a395a8b9939bd48858efbd3da5897a59d4697eba (patch) | |
tree | 350aa431f224e733f6da811ebdfb9ad25534d9ce /epan/maxmind_db.c | |
parent | 02085c80ab10507f6a75d75fe1e43facade98a0e (diff) |
GeoIP: avoid closing random file descriptors
Previously there were three different pipe validity checks:
PID != WS_INVALID_PID, PID != 0 and stdin != 0. This resulted in
using/closing file descriptors which might be owned by something else.
When no GeoIP databases are defined, mmdb_resolve_stop would be called
to close the pipe and set PID to WS_INVALID_PID. stdin is however not
cleared and future invocations would try to close the previous fd.
Change-Id: I1d15da29208efb41098ee6a4edeeabf61f84c2b3
Fixes: v2.5.1rc0-466-ga1da75c554 ("Transition from GeoIP Legacy to MaxMindDB.")
Reviewed-on: https://code.wireshark.org/review/26391
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Diffstat (limited to 'epan/maxmind_db.c')
-rw-r--r-- | epan/maxmind_db.c | 25 |
1 files changed, 19 insertions, 6 deletions
diff --git a/epan/maxmind_db.c b/epan/maxmind_db.c index ece13f819d..84fbb3a420 100644 --- a/epan/maxmind_db.c +++ b/epan/maxmind_db.c @@ -221,7 +221,10 @@ process_mmdbr_stdout(int fd) { * Stop our mmdbresolve process. */ static void mmdb_resolve_stop(void) { - if (!mmdbr_pipe.pid) return; + if (!ws_pipe_valid(&mmdbr_pipe)) { + MMDB_DEBUG("not cleaning up, invalid PID %d", mmdbr_pipe.pid); + return; + } ws_close(mmdbr_pipe.stdin_fd); MMDB_DEBUG("closing pid %d", mmdbr_pipe.pid); @@ -244,10 +247,18 @@ static void mmdb_resolve_start(void) { mmdb_str_chunk = wmem_map_new(wmem_epan_scope(), wmem_str_hash, g_str_equal); } - if (!mmdb_file_arr) return; + if (!mmdb_file_arr) { + MMDB_DEBUG("unexpected mmdb_file_arr == NULL"); + return; + } mmdb_resolve_stop(); + if (mmdb_file_arr->len == 0) { + MMDB_DEBUG("no GeoIP databases found"); + return; + } + GPtrArray *args = g_ptr_array_new(); char *mmdbresolve = g_strdup_printf("%s%c%s", get_progfile_dir(), G_DIR_SEPARATOR, "mmdbresolve"); g_ptr_array_add(args, mmdbresolve); @@ -262,7 +273,9 @@ static void mmdb_resolve_start(void) { MMDB_DEBUG("spawned %s pid %d", mmdbresolve, pipe_pid); for (guint i = 0; i < args->len; i++) { - g_free(g_ptr_array_index(args, i)); + char *arg = g_ptr_array_index(args, i); + MMDB_DEBUG("args: %s", arg); + g_free(arg); } g_ptr_array_free(args, TRUE); @@ -405,7 +418,7 @@ void maxmind_db_pref_cleanup(void) gboolean maxmind_db_lookup_process(void) { - if (mmdbr_pipe.pid == WS_INVALID_PID) return FALSE; + if (!ws_pipe_valid(&mmdbr_pipe)) return FALSE; return process_mmdbr_stdout(mmdbr_pipe.stdout_fd); } @@ -415,7 +428,7 @@ maxmind_db_lookup_ipv4(guint32 addr) { mmdb_lookup_t *result = (mmdb_lookup_t *) wmem_map_lookup(mmdb_ipv4_map, GUINT_TO_POINTER(addr)); if (!result) { - if (mmdbr_pipe.stdin_fd) { + if (ws_pipe_valid(&mmdbr_pipe)) { char addr_str[WS_INET_ADDRSTRLEN + 1]; ws_inet_ntop4(&addr, addr_str, WS_INET_ADDRSTRLEN); MMDB_DEBUG("looking up %s", addr_str); @@ -439,7 +452,7 @@ maxmind_db_lookup_ipv6(const ws_in6_addr *addr) { mmdb_lookup_t * result = (mmdb_lookup_t *) wmem_map_lookup(mmdb_ipv6_map, addr->bytes); if (!result) { - if (mmdbr_pipe.stdin_fd) { + if (ws_pipe_valid(&mmdbr_pipe)) { char addr_str[WS_INET6_ADDRSTRLEN + 1]; ws_inet_ntop6(addr, addr_str, WS_INET6_ADDRSTRLEN); MMDB_DEBUG("looking up %s", addr_str); |