[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wireshark-bugs
Subject:    [Wireshark-bugs] [Bug 13864] New: Memory leaks related to interface_options and interface_t types (i
From:       bugzilla-daemon () wireshark ! org
Date:       2017-06-29 23:27:34
Message-ID: bug-13864-15 () https ! bugs ! wireshark ! org/bugzilla/
[Download RAW message or body]

--14987788551.6FebF8A59.12353
Date: Thu, 29 Jun 2017 23:27:35 +0000
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: https://bugs.wireshark.org/bugzilla/
Auto-Submitted: auto-generated

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=13864

            Bug ID: 13864
           Summary: Memory leaks related to interface_options and
                    interface_t types (interface lists)
           Product: Wireshark
           Version: Git
          Hardware: All
                OS: All
            Status: CONFIRMED
          Severity: Normal
          Priority: Low
         Component: Qt UI
          Assignee: bugzilla-admin@wireshark.org
          Reporter: peter@lekensteyn.nl
  Target Milestone: ---

Created attachment 15665
  --> https://bugs.wireshark.org/bugzilla/attachment.cgi?id=15665&action=edit
ASAN log file showing memory leaks

Build Information:
Wireshark 2.5.0 (v2.5.0rc0-319-ga5ce639e00)                                     

Copyright 1998-2017 Gerald Combs <gerald@wireshark.org> and contributors.       
License GPLv2+: GNU GPL version 2 or later
<http://www.gnu.org/licenses/old-licenses/gpl-2.0.html>                         
This is free software; see the source for copying conditions. There is NO       
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.     

Compiled (64-bit) with Qt 5.9.0, with libpcap, with POSIX capabilities (Linux), 
with libnl 3, with GLib 2.52.2, with zlib 1.2.11, without SMI, with c-ares      
1.12.0, with Lua 5.2.4, with GnuTLS 3.5.13, with Gcrypt 1.7.7, with MIT         
Kerberos, with GeoIP, with nghttp2 1.23.1, with LZ4, with Snappy, with libxml2  
2.9.4, with QtMultimedia, without AirPcap, with SBC, with SpanDSP.              

Running on Linux 4.11.6-1-ARCH, with Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz  
(with SSE4.2), with 31996 MB of physical memory, with locale en_GB.UTF-8, with  
libpcap version 1.8.1, with GnuTLS 3.5.13, with Gcrypt 1.7.7, with zlib 1.2.11. 

Built using clang 4.2.1 Compatible Clang 4.0.0 (tags/RELEASE_400/final).        
--
There are several memory leaks related to these types:
- interface_options - interface options to use for the next capture
- interface_t - known interface information

One of the problems is that the current handling of interfaces is hard to
understand. Memory ownership is not clearly defined, structures are copied,
modified and written back; see for example in ui/iface_lists.c, function
hide_interface:

 interface_t device;
 device = g_array_index(global_capture_opts.all_ifaces, interface_t, i);
 /* modify device */
 global_capture_opts.all_ifaces =
g_array_remove_index(global_capture_opts.all_ifaces, i);
 g_array_insert_val(global_capture_opts.all_ifaces, i, device);

Why?

There exists functions like capture_opts_del_iface (to cleanup
interface_options) and capture_opts_free_interface_t (to cleanup interface_t),
but this is not used consistently.

See the attached ASAN report for a bunch of leaks from scan_local_interfaces.
It is based on:
1f44007dd3 (master) Added General Notification Message opcode to CFM
1978bc86bf dumpcap: fix buffer overflow on packets larger than 2048 bytes
f3dba31faa dumpcap: fix minor memory leak at begin of capture
a5ce639e00 extcap: another round of memory leak fixes

I came up with this attempt to fix some memleaks, but it does not solve the
root problem:

diff --git a/ui/qt/interface_tree_cache_model.cpp
b/ui/qt/interface_tree_cache_model.cpp
index 3b9f6d56b6..0dd28fc2f7 100644
--- a/ui/qt/interface_tree_cache_model.cpp
+++ b/ui/qt/interface_tree_cache_model.cpp
@@ -598,5 +598,7 @@ void InterfaceTreeCacheModel::deleteDevice(const
QModelIndex &index)
     else
     {
+        interface_t device = g_array_index(global_capture_opts.all_ifaces,
interface_t, row);
         g_array_remove_index(global_capture_opts.all_ifaces, row);
+        capture_opts_free_interface_t(&device);
         emit endRemoveRows();
         wsApp->emitAppSignal(WiresharkApplication::LocalInterfacesChanged);
diff --git a/ui/qt/manage_interfaces_dialog.cpp
b/ui/qt/manage_interfaces_dialog.cpp
index 19b0d312d9..a8dae65bbd 100644
--- a/ui/qt/manage_interfaces_dialog.cpp
+++ b/ui/qt/manage_interfaces_dialog.cpp
@@ -498,7 +498,7 @@ void ManageInterfacesDialog::on_delRemote_clicked()
             continue;
         global_capture_opts.all_ifaces =
g_array_remove_index(global_capture_opts.all_ifaces, i);
+        capture_opts_free_interface_t(device);
     }
     delete item;
-    fflush(stdout); // ???
 }

-- 
You are receiving this mail because:
You are watching all bug changes.
--14987788551.6FebF8A59.12353
Date: Thu, 29 Jun 2017 23:27:35 +0000
MIME-Version: 1.0
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: https://bugs.wireshark.org/bugzilla/
Auto-Submitted: auto-generated

<html>
    <head>
      <base href="https://bugs.wireshark.org/bugzilla/" />
      <style>
        body, th, td {
            font-size: 12px;
            font-family: Arial, Helvetica, sans-serif; }
        p, pre { margin-top: 1em; }
        pre {
            font-family: Bitstream Vera Sans Mono, Consolas, Lucida Console, \
monospace;  white-space: pre-wrap;
	}
        table { border: 0; border-spacing: 0; border-collapse: collapse; }
        th, td {
            padding: 0.25em;
            padding-left: 0.5em;
            padding-right: 0.5em;
        }
        th { background: rgb(240, 240, 240); }
        th.th_top { border-bottom: 1px solid rgb(116, 126, 147); }
        th.th_left { border-right: 1px solid rgb(116, 126, 147); }
        td.removed { background-color: #ffcccc; }
        td.added { background-color: #e4ffc7; }
      </style>
    </head>
    <body><table>
        <tr>
          <th class="th_left">Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_CONFIRMED "
   title="CONFIRMED - Memory leaks related to interface_options and interface_t types \
(interface lists)"  href="https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=13864">13864</a>
  </td>
        </tr>

        <tr>
          <th class="th_left">Summary</th>
          <td>Memory leaks related to interface_options and interface_t types \
(interface lists)  </td>
        </tr>

        <tr>
          <th class="th_left">Product</th>
          <td>Wireshark
          </td>
        </tr>

        <tr>
          <th class="th_left">Version</th>
          <td>Git
          </td>
        </tr>

        <tr>
          <th class="th_left">Hardware</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th class="th_left">OS</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th class="th_left">Status</th>
          <td>CONFIRMED
          </td>
        </tr>

        <tr>
          <th class="th_left">Severity</th>
          <td>Normal
          </td>
        </tr>

        <tr>
          <th class="th_left">Priority</th>
          <td>Low
          </td>
        </tr>

        <tr>
          <th class="th_left">Component</th>
          <td>Qt UI
          </td>
        </tr>

        <tr>
          <th class="th_left">Assignee</th>
          <td>bugzilla-admin&#64;wireshark.org
          </td>
        </tr>

        <tr>
          <th class="th_left">Reporter</th>
          <td>peter&#64;lekensteyn.nl
          </td>
        </tr>

        <tr>
          <th class="th_left">Target Milestone</th>
          <td>---
          </td>
        </tr></table>
      <p>
        <div>
        <pre>Created <span class=""><a href="attachment.cgi?id=15665" \
name="attach_15665" title="ASAN log file showing memory leaks">attachment 15665</a> \
<a href="attachment.cgi?id=15665&amp;action=edit" title="ASAN log file showing memory \
leaks">[details]</a></span> ASAN log file showing memory leaks

Build Information:
Wireshark 2.5.0 (v2.5.0rc0-319-<a \
href="https://code.wireshark.org/review/#/q/commit:a5ce639e00">ga5ce639e00</a>)       \


Copyright 1998-2017 Gerald Combs &lt;<a \
href="mailto:gerald&#64;wireshark.org">gerald&#64;wireshark.org</a>&gt; and \
contributors.        License GPLv2+: GNU GPL version 2 or later
&lt;<a href="http://www.gnu.org/licenses/old-licenses/gpl-2.0.html">http://www.gnu.org/licenses/old-licenses/gpl-2.0.html</a>&gt; \
 This is free software; see the source for copying conditions. There is NO       
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.     

Compiled (64-bit) with Qt 5.9.0, with libpcap, with POSIX capabilities (Linux), 
with libnl 3, with GLib 2.52.2, with zlib 1.2.11, without SMI, with c-ares      
1.12.0, with Lua 5.2.4, with GnuTLS 3.5.13, with Gcrypt 1.7.7, with MIT         
Kerberos, with GeoIP, with nghttp2 1.23.1, with LZ4, with Snappy, with libxml2  
2.9.4, with QtMultimedia, without AirPcap, with SBC, with SpanDSP.              

Running on Linux 4.11.6-1-ARCH, with Intel(R) Core(TM) i7-6700HQ CPU &#64; 2.60GHz  
(with SSE4.2), with 31996 MB of physical memory, with locale en_GB.UTF-8, with  
libpcap version 1.8.1, with GnuTLS 3.5.13, with Gcrypt 1.7.7, with zlib 1.2.11. 

Built using clang 4.2.1 Compatible Clang 4.0.0 (tags/RELEASE_400/final).        
--
There are several memory leaks related to these types:
- interface_options - interface options to use for the next capture
- interface_t - known interface information

One of the problems is that the current handling of interfaces is hard to
understand. Memory ownership is not clearly defined, structures are copied,
modified and written back; see for example in ui/iface_lists.c, function
hide_interface:

 interface_t device;
 device = g_array_index(global_capture_opts.all_ifaces, interface_t, i);
 /* modify device */
 global_capture_opts.all_ifaces =
g_array_remove_index(global_capture_opts.all_ifaces, i);
 g_array_insert_val(global_capture_opts.all_ifaces, i, device);

Why?

There exists functions like capture_opts_del_iface (to cleanup
interface_options) and capture_opts_free_interface_t (to cleanup interface_t),
but this is not used consistently.

See the attached ASAN report for a bunch of leaks from scan_local_interfaces.
It is based on:
1f44007dd3 (master) Added General Notification Message opcode to CFM
1978bc86bf dumpcap: fix buffer overflow on packets larger than 2048 bytes
f3dba31faa dumpcap: fix minor memory leak at begin of capture
a5ce639e00 extcap: another round of memory leak fixes

I came up with this attempt to fix some memleaks, but it does not solve the
root problem:

diff --git a/ui/qt/interface_tree_cache_model.cpp
b/ui/qt/interface_tree_cache_model.cpp
index 3b9f6d56b6..0dd28fc2f7 100644
--- a/ui/qt/interface_tree_cache_model.cpp
+++ b/ui/qt/interface_tree_cache_model.cpp
&#64;&#64; -598,5 +598,7 &#64;&#64; void InterfaceTreeCacheModel::deleteDevice(const
QModelIndex &amp;index)
     else
     {
+        interface_t device = g_array_index(global_capture_opts.all_ifaces,
interface_t, row);
         g_array_remove_index(global_capture_opts.all_ifaces, row);
+        capture_opts_free_interface_t(&amp;device);
         emit endRemoveRows();
         wsApp-&gt;emitAppSignal(WiresharkApplication::LocalInterfacesChanged);
diff --git a/ui/qt/manage_interfaces_dialog.cpp
b/ui/qt/manage_interfaces_dialog.cpp
index 19b0d312d9..a8dae65bbd 100644
--- a/ui/qt/manage_interfaces_dialog.cpp
+++ b/ui/qt/manage_interfaces_dialog.cpp
&#64;&#64; -498,7 +498,7 &#64;&#64; void \
ManageInterfacesDialog::on_delRemote_clicked()  continue;
         global_capture_opts.all_ifaces =
g_array_remove_index(global_capture_opts.all_ifaces, i);
+        capture_opts_free_interface_t(device);
     }
     delete item;
-    fflush(stdout); // ???
 }</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are watching all bug changes.</li>
      </ul>
    </body>
</html>
--14987788551.6FebF8A59.12353--


[Attachment #3 (text/plain)]

___________________________________________________________________________
Sent via:    Wireshark-bugs mailing list <wireshark-bugs@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-bugs
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-bugs
             mailto:wireshark-bugs-request@wireshark.org?subject=unsubscribe

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic