Details

      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.

        Activity

        Hide
        Andris Zeila added a comment -

        (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.

        Show
        Andris Zeila added a comment - (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.
        Hide
        Andris Zeila added a comment -

        Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10575

        Show
        Andris Zeila added a comment - Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10575
        Hide
        Andris Zeila added a comment -

        Released in:

        • pre-3.0.2rc1 r59331
        • pre-3.1.0 r59332
        Show
        Andris Zeila added a comment - Released in: pre-3.0.2rc1 r59331 pre-3.1.0 r59332

          People

          • Assignee:
            Unassigned
            Reporter:
            Andris Zeila
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: