After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 724900 - ensure_internal_ports() doesn't deep copy port array
ensure_internal_ports() doesn't deep copy port array
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: ModemManager
unspecified
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-02-21 15:58 UTC by Dan Williams
Modified: 2014-02-27 19:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
deep copy ports array (2.06 KB, patch)
2014-02-24 19:18 UTC, Dan Williams
none Details | Review
deep copy ports array (2.11 KB, patch)
2014-02-24 19:20 UTC, Dan Williams
none Details | Review

Description Dan Williams 2014-02-21 15:58:56 UTC
ensure_internal_ports():

    *dup_ports = g_malloc (sizeof (MMModemPortInfo) * self->priv->ports->len);
    memcpy (*dup_ports, self->priv->ports->data, sizeof (MMModemPortInfo) * self->priv->ports->len);

Unfortunately, self->priv->ports is a GArray of MMModemPortInfo structures, which have the layout:

struct _MMModemPortInfo {
    gchar *name;
    MMModemPortType type;
};

ensure_internal_ports() only shallow-copies the structures, and fails to copy the 'name' item to the duplicated array.  This means that mm_modem_port_info_array_free() actually frees the port names held internally by the MMModem object, and leads to use-after-free if you call mm_modem_get_ports() more than once.
Comment 1 Dan Williams 2014-02-24 19:18:12 UTC
Created attachment 270175 [details] [review]
deep copy ports array

How about this patch? (untested)
Comment 2 Dan Williams 2014-02-24 19:20:41 UTC
Created attachment 270176 [details] [review]
deep copy ports array

This version seems clearer about the destination array.
Comment 3 Thomas Haller 2014-02-25 13:12:18 UTC
Looks good
Comment 4 Dan Williams 2014-02-27 19:05:09 UTC
Tested and merged to git master.