GNOME Bugzilla – Bug 746195
tracker crashing within gnome-documents search provider
Last modified: 2015-10-30 16:18:30 UTC
we are getting lots of reports of this one, with tracker 1.3.5 https://bugs.launchpad.net/bugs/1432098/+attachment/4344952/+files/ThreadStacktrace.txt
Hi, can you please check the link, shows page not found for me
oh bug was still marked private, should work now. Though here is the backtrace .
+ Trace 234841
Thread 3 (Thread 0x7f2501ce7700 (LWP 11654))
Thread 2 (Thread 0x7f25024e8700 (LWP 11653))
Thread 1 (Thread 0x7f2502ce9700 (LWP 11652))
Interesting. Is it reproduced calling that SPARQL query? Or is there somethign special to show this bug? A few observations: 1. The cancellable is 0x2, that looks potentially like memory corruption without looking at the GCancellable code that is. 2. That code hasn't changed in over 4 years, I wonder if something underneath us has changed? I know that the g_io_scheduler_push_job() stuff is deprecated and needs updating ASAP - actually I've asked Jürg about that and it was problematic when we last spoke.
Created attachment 310856 [details] Trace There is this bug report with malloc(): memory corruption (fast) https://bugs.launchpad.net/ubuntu-gnome/+bug/1432068. Backtrace attached.
This issue is also seen on Debian. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=794646
I can easily reproduce this crash: - in one terminal run /usr/bin/gnome-documents --gapplication-service - within 10 seconds of the above, in another terminal run dbus-send --print-reply --dest=org.gnome.Documents /org/gnome/Documents/SearchProvider org.gnome.Shell.SearchProvider2.GetInitialResultSet array:string:"search" When I do that under valgrind, I get this: ==23172== Thread 8 pool: ==23172== Invalid write of size 1 ==23172== at 0x174A56C1: tracker_parser_unaccent_nfkd_string (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-common.so.0.0.0) ==23172== by 0x1726AA02: function_sparql_unaccent (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-data.so.0.0.0) ==23172== by 0x1791D6EE: sqlite3VdbeExec (in /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6) ==23172== by 0x17926826: sqlite3_step (in /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6) ==23172== by 0x1726B2FF: db_cursor_iter_next (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-data.so.0.0.0) ==23172== by 0x1726BAB6: tracker_db_cursor_iter_next_thread (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-data.so.0.0.0) ==23172== by 0x70A68FE: run_in_thread (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4600.1) ==23172== by 0x7092985: io_job_thread (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4600.1) ==23172== by 0x70B7D87: g_task_thread_pool_thread (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4600.1) ==23172== by 0x50FC2FD: g_thread_pool_thread_proxy (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4600.1) ==23172== by 0x50FB964: g_thread_proxy (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4600.1) ==23172== by 0x5E706A9: start_thread (pthread_create.c:333) ==23172== Address 0x14072b52 is 0 bytes after a block of size 2 alloc'd ==23172== at 0x4C2DD9F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==23172== by 0x17B9D516: u8_normalize (in /usr/lib/x86_64-linux-gnu/libunistring.so.0.1.2) ==23172== by 0x1726A9F4: function_sparql_unaccent (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-data.so.0.0.0) ==23172== by 0x1791D6EE: sqlite3VdbeExec (in /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6) ==23172== by 0x17926826: sqlite3_step (in /usr/lib/x86_64-linux-gnu/libsqlite3.so.0.8.6) ==23172== by 0x1726B2FF: db_cursor_iter_next (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-data.so.0.0.0) ==23172== by 0x1726BAB6: tracker_db_cursor_iter_next_thread (in /usr/lib/x86_64-linux-gnu/tracker-1.0/libtracker-data.so.0.0.0) ==23172== by 0x70A68FE: run_in_thread (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4600.1) ==23172== by 0x7092985: io_job_thread (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4600.1) ==23172== by 0x70B7D87: g_task_thread_pool_thread (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4600.1) ==23172== by 0x50FC2FD: g_thread_pool_thread_proxy (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4600.1) ==23172== by 0x50FB964: g_thread_proxy (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4600.1) When I look at what's happening under gdb, I see function_sparql_unaccent getting called with a string "10": (gdb) info locals zInput = 0x7fffc80b83d8 "10" zOutput = <optimized out> written = 0 nInput = 2 Then u8_normalize() gets called and allocates a 2-byte buffer: (gdb) info locals zInput = 0x7fffc80b83d8 "10" zOutput = 0x7fffc80fac50 "10" written = 2 nInput = <optimized out> Then tracker_parser_unaccent_nfkd_string from tracker-parser-libunistring.c:164 gets called and helpfully writes a '\0' past the end of the buffer (tracker-parser-libunistring.c:213 in current git).
Created attachment 313923 [details] [review] fix buffer overrun in libunistring version of tracker_parser_unaccent_nfkd_string() I've tested this patch locally, and I no longer have the crash. Valgrind also doesn't complain any more (I'm ignoring warnings about uninitalized memory reads in libmozjs).
Citations about the libunistring memory representation: * https://www.gnu.org/software/libunistring/manual/libunistring.html#In_002dmemory-representation * https://www.gnu.org/software/libunistring/manual/libunistring.html#Unicode-strings Since u8_normalize() passes around pairs of (const uint8_t*, size_t), I think it's safe to assume it uses the 2nd of the formats (no trailing NUL). Also, Valgrind reported that a 2-byte buffer was allocated for a 2-character string, so. The buffer allocated by u8_normalize() is passed directly to tracker_parser_unaccent_nfkd_string(), so the latter must not assume there's space for a trailing NUL. And that's why I think my patch is correct.
Created attachment 313924 [details] [review] fix buffer overrun in libunistring builds of tracker_parser_unaccent_nfkd_string() Version 2: remove useless assertion (gsize is unsigned, it can't be < 0).
Created attachment 313933 [details] [review] fix buffer overrun in libunistring builds of tracker_parser_unaccent_nfkd_string() [v3] Version 3: add a NUL terminator in one place only; tracker_parser_message_hex() doesn't need it.
Review of attachment 313933 [details] [review]: The logic looks good in this one. The libunistring version of tracker_parser_unaccent_nfkd_string() doesn'tt rely on being NUL-terminated and therefore it shouldn't explicitly force it.
Comment on attachment 313933 [details] [review] fix buffer overrun in libunistring builds of tracker_parser_unaccent_nfkd_string() [v3] Indeed, looks good :). Please push to master/tracker-1.6/tracker-1.4.
Comment on attachment 313933 [details] [review] fix buffer overrun in libunistring builds of tracker_parser_unaccent_nfkd_string() [v3] Pushed to master, all the way down to tracker-1.2
*** Bug 752556 has been marked as a duplicate of this bug. ***
*** Bug 755560 has been marked as a duplicate of this bug. ***