[ZBX-10575] History cache memory leak Created: 2016 Mar 24 Updated: 2017 May 30 Resolved: 2016 Apr 08 |
|
| Status: | Closed |
| Project: | ZABBIX BUGS AND ISSUES |
| Component/s: | Proxy (P), Server (S) |
| Affects Version/s: | None |
| Fix Version/s: | 3.0.2rc1, 3.2.0alpha1 |
| Type: | Incident report | Priority: | Major |
| Reporter: | Andris Zeila | Assignee: | Unassigned |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | memoryleak | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
DCcalculate_item_delta_float() function generates error message if the resulting value is out of acceptable double value range. This message is stored in ZBX_DC_HISTORY:value_orig.err field which is not freed. Before history cache changes all ZBX_DC_HISTORY text data was allocated on heap and properly freed afterwards. Now the ZBX_DC_HISTORY error and text fields are references to the history cache shared memory and shouldn't be freed. The simplest (but not cleanest). solution would be to add ownership flags to the ZBX_DC_HISTORY. Anoher solution would be to duplicate all ZBX_DC_HISTORY text values on heap. There would be some overhead, but it looks more robust solution. |
| Comments |
| Comment by Andris Zeila [ 2016 Mar 24 ] |
|
(1) The lastlogsize/mtime fields are always copied, even if the meta flag is not set and the data contains random values. While it doesn't look dangerous currently it would be better to fix it:
Index: src/libs/zbxdbcache/dbcache.c
===================================================================
--- src/libs/zbxdbcache/dbcache.c (revision 58985)
+++ src/libs/zbxdbcache/dbcache.c (working copy)
@@ -2855,8 +2855,12 @@
}
(*data)->value_type = item_value->value_type;
- (*data)->lastlogsize = item_value->lastlogsize;
- (*data)->mtime = item_value->mtime;
+
+ if (0 != (ZBX_DC_FLAG_META & item_value->flags))
+ {
+ (*data)->lastlogsize = item_value->lastlogsize;
+ (*data)->mtime = item_value->mtime;
+ }
return SUCCEED;
}
Also because of this assignment valgrind gives strange errors: ==2870== at 0x48265F: mem_ptr_to_prev_field (memalloc.c:209) ==2870== by 0x4827DB: mem_unlink_chunk (memalloc.c:244) ==2870== by 0x4829E5: __mem_malloc (memalloc.c:305) ==2870== by 0x4839E0: __zbx_mem_malloc (memalloc.c:690) ==2870== by 0x468D20: __hc_mem_malloc_func (dbcache.c:2561) ==2870== by 0x4691B0: hc_clone_history_data (dbcache.c:2796) ==2870== by 0x46952B: hc_add_item_values (dbcache.c:2899) ==2870== by 0x468BF0: dc_flush_history (dbcache.c:2545) ==2870== by 0x42015C: get_values (poller.c:802) ==2870== by 0x4202F2: poller_thread (poller.c:852) ==2870== by 0x492781: zbx_thread_start (threads.c:128) ==2870== by 0x4181C9: MAIN_ZABBIX_ENTRY (server.c:947) ==2870== Uninitialised value was created by a heap allocation ==2870== at 0x4C28C20: malloc (vg_replace_malloc.c:296) ==2870== by 0x4C2AFCF: realloc (vg_replace_malloc.c:692) ==2870== by 0x497A3E: zbx_realloc2 (misc.c:352) ==2870== by 0x468078: dc_local_get_history_slot (dbcache.c:2226) ==2870== by 0x46829B: dc_local_add_history_str (dbcache.c:2286) ==2870== by 0x468AD6: dc_add_history (dbcache.c:2513) ==2870== by 0x41F927: get_values (poller.c:714) ==2870== by 0x4202F2: poller_thread (poller.c:852) ==2870== by 0x492781: zbx_thread_start (threads.c:128) ==2870== by 0x4181C9: MAIN_ZABBIX_ENTRY (server.c:947) ==2870== by 0x490CE7: daemon_start (daemon.c:392) ==2870== by 0x417C88: main (server.c:757) ==2870== ==2870== Conditional jump or move depends on uninitialised value(s) ==2870== at 0x4826DE: mem_link_chunk (memalloc.c:223) ==2870== by 0x482A82: __mem_malloc (memalloc.c:324) ==2870== by 0x4839E0: __zbx_mem_malloc (memalloc.c:690) ==2870== by 0x468D20: __hc_mem_malloc_func (dbcache.c:2561) ==2870== by 0x4691B0: hc_clone_history_data (dbcache.c:2796) ==2870== by 0x46952B: hc_add_item_values (dbcache.c:2899) ==2870== by 0x468BF0: dc_flush_history (dbcache.c:2545) ==2870== by 0x42015C: get_values (poller.c:802) ==2870== by 0x4202F2: poller_thread (poller.c:852) ==2870== by 0x492781: zbx_thread_start (threads.c:128) ==2870== by 0x4181C9: MAIN_ZABBIX_ENTRY (server.c:947) ==2870== by 0x490CE7: daemon_start (daemon.c:392) ==2870== Uninitialised value was created by a heap allocation ==2870== at 0x4C28C20: malloc (vg_replace_malloc.c:296) ==2870== by 0x4C2AFCF: realloc (vg_replace_malloc.c:692) ==2870== by 0x497A3E: zbx_realloc2 (misc.c:352) ==2870== by 0x468078: dc_local_get_history_slot (dbcache.c:2226) ==2870== by 0x46829B: dc_local_add_history_str (dbcache.c:2286) ==2870== by 0x468AD6: dc_add_history (dbcache.c:2513) ==2870== by 0x41F927: get_values (poller.c:714) ==2870== by 0x4202F2: poller_thread (poller.c:852) ==2870== by 0x492781: zbx_thread_start (threads.c:128) ==2870== by 0x4181C9: MAIN_ZABBIX_ENTRY (server.c:947) ==2870== by 0x490CE7: daemon_start (daemon.c:392) ==2870== by 0x417C88: main (server.c:757) The uninitialized code is in the middle of a chunk and should not affect memory allocator, but valgrind complains when allocator accesses size fields that are located before and after allocated memory chunk. |
| Comment by Andris Zeila [ 2016 Apr 05 ] |
|
Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10575 |
| Comment by Andris Zeila [ 2016 Apr 07 ] |
|
Released in:
|