GoogleCodeExporter
commented
9 years ago revision 10529Original comment by vanya...@gmail.com on 12 Feb 2015 at 11:22
Open GoogleCodeExporter opened 9 years ago
GoogleCodeExporter
commented
9 years ago revision 10529Original comment by vanya...@gmail.com on 12 Feb 2015 at 11:22
GoogleCodeExporter
commented
9 years ago that looks like a false positive.
It looks like an optimistic read, which is common in optimizations, especially
in string handling. Note how it's 0 bytes past the end of the block of country
names in GeoIP.c. it's as if the 3 byte string was read with a 4 byte (full
word) read. It's a fairly typical optimization since reading off the end of
your buffer is OK, and page boundaries won't be crossed with anyway, since it's
an aligned read.
Is there a way to tell address sanitizer to suppress this particular read?
otherwise, you could also turn off geoip, to not trigger this.Original comment by arvid.no...@gmail.com on 13 Feb 2015 at 4:17
GoogleCodeExporter
commented
9 years ago > It's a fairly typical optimization since reading off the end of your buffer
is OK, and page boundaries won't be crossed with anyway, since it's an aligned
read.
Address sanitizer is special compiler option, not binary instrumentation. Also
authors claim there is no false positives.
http://static.googleusercontent.com/media/research.google.com/en//pubs/archive/3
7752.pdf -- see 3.6.
Assuming there is no false positives this is either bug in address sanitizer or
GeoIP.Original comment by vanya...@gmail.com on 13 Feb 2015 at 8:58
GoogleCodeExporter
commented
9 years ago Could _GeoIP_seek_record returns index that is out of bounds of
GeoIP_country_code?Original comment by vanya...@gmail.com on 13 Feb 2015 at 9:02
GoogleCodeExporter
commented
9 years ago 0x0000014073f7 is located 0 bytes to the right of global variable
'GeoIP_country_code (GeoIP.c)' (0x1407100) of size 759
759 = 253 * 3
/home/ivan/d/libtorrent-svn-1_0/src/peer_connection.cpp:236
m_country[0] = country[0];
If I understand correctly this error happened because _GeoIP_seek_record
returned 253.Original comment by vanya...@gmail.com on 13 Feb 2015 at 9:07
GoogleCodeExporter
commented
9 years ago I have made a very simple patch to test my hypothesis:
diff --git a/src/GeoIP.c b/src/GeoIP.c
index cb4437e..f868e80 100644
--- a/src/GeoIP.c
+++ b/src/GeoIP.c
@@ -788,6 +788,11 @@ const char *GeoIP_country_name_by_ipnum (GeoIP* gi,
unsigned long ipnum) {
const char *GeoIP_country_code_by_ipnum (GeoIP* gi, unsigned long ipnum) {
int country_id;
country_id = GeoIP_id_by_ipnum(gi, ipnum);
+ if (country_id >= 253)
+ {
+ fprintf(stderr, "country_id is out of range %d, ip: %lu\n",
country_id, ipnum);
+ return NULL;
+ }
return (country_id > 0) ? GeoIP_country_code[country_id] : NULL;
}
diff --git a/src/session_impl.cpp b/src/session_impl.cpp
index ff65680..db07478 100644
--- a/src/session_impl.cpp
+++ b/src/session_impl.cpp
@@ -1441,6 +1441,16 @@ namespace aux {
TORRENT_ASSERT(is_network_thread());
if (!a.is_v4() || m_country_db == 0) return 0;
+
+ for (size_t i = 0; i != 100; ++i)
+ {
+ unsigned a = rand() % 256;
+ unsigned b = rand() % 256;
+ unsigned c = rand() % 256;
+ unsigned d = rand() % 256;
+ GeoIP_country_code_by_ipnum(m_country_db, (a << 24) |
(b << 16) | (c << 8) | d);
+ }
+
return GeoIP_country_code_by_ipnum(m_country_db, a.to_v4().to_ulong());
}
Almost instantly after I started qbittorrent I've got an error:
country_id is out of range 254, ip: 693073990
If I do my arithmetic right it corresponds to ip address 41.79.120.70 (or
70.120.79.41, not sure).
Original comment by vanya...@gmail.com on 16 Feb 2015 at 12:59
GoogleCodeExporter
commented
9 years ago Yes. There are lots of them:
country_id is out of range 253, ip: 3194056757
country_id is out of range 253, ip: 3131009708
country_id is out of range 254, ip: 702434290
country_id is out of range 253, ip: 3362477991
country_id is out of range 253, ip: 3187950232
Original comment by vanya...@gmail.com on 16 Feb 2015 at 1:02
GoogleCodeExporter
commented
9 years ago would you mind reporting this to maxmind? It probably makes sense to grab their
latest version of GeoIP.c first, to confirm it still happens in there.
It probably makes sense to update the libtorrent version to the latest on
github anyway.
https://github.com/maxmind/geoip-api-c/tree/master/libGeoIP
It looks like they may have fixed this issue though, by failing with an error
"invalid database".Original comment by arvid.no...@gmail.com on 16 Feb 2015 at 1:26
GoogleCodeExporter
commented
9 years ago I've reported a issue https://github.com/maxmind/geoip-api-c/issues/53
I still don't see why the same error couldn't occur on corrupted database in
the latest version.
Original comment by vanya...@gmail.com on 16 Feb 2015 at 8:48
Original issue reported on code.google.com by
vanya...@gmail.comon 12 Feb 2015 at 11:21