fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2025-12-28 19:10:09 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

libnm-glib: fix use-after-free getting device vendor and product

Browse source 
Because _device_update_description() freed and update *both* the
vendor and product description, if the function was not able to
read one of them the first time, it would throw both away the second
time and try re-read them.  That caused code like this:

vendor = nm_device_get_vendor (device);
product = nm_device_get_product (device);

to be left with a freed 'vendor' value if _device_update_description()
could not read the product when first called from nm_device_get_vendor(),
because it would be called a second time from nm_device_get_product()
and free priv->vendor and priv->product and then attempt to re-read them.

Fix this by making the function return only one value so that the
callers can control the liftime of the property they are trying to set.

==29355== Invalid read of size 8
==29355==    at 0x38F7289840: __GI_mempcpy (memcpy.S:122)
==29355==    by 0x38F7276F21: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1353)
==29355==    by 0x38F7247DA6: vfprintf (vfprintf.c:1615)
==29355==    by 0x38F7250E28: printf (printf.c:34)
==29355==    by 0x41E79F: print_fields (utils.c:351)
==29355==    by 0x414CAB: show_device_info (devices.c:636)
==29355==    by 0x415CE8: do_devices (devices.c:1094)
==29355==    by 0x41D9A9: start (nmcli.c:121)
==29355==    by 0x38F6E47A54: g_main_context_dispatch (gmain.c:2715)
==29355==    by 0x38F6E47D87: g_main_context_iterate.isra.24 (gmain.c:3290)
==29355==    by 0x38F6E48181: g_main_loop_run (gmain.c:3484)
==29355==    by 0x40CB89: main (nmcli.c:359)
==29355==  Address 0x50a0401 is 1 bytes inside a block of size 18 free'd
==29355==    at 0x4A077E6: free (vg_replace_malloc.c:446)
==29355==    by 0x38F6E4D79E: g_free (gmem.c:252)
==29355==    by 0x4C55F53: _device_update_description (nm-device.c:1417)
==29355==    by 0x4C56D26: nm_device_get_product (nm-device.c:1502)
==29355==    by 0x414B16: show_device_info (devices.c:620)
==29355==    by 0x415CE8: do_devices (devices.c:1094)
==29355==    by 0x41D9A9: start (nmcli.c:121)
==29355==    by 0x38F6E47A54: g_main_context_dispatch (gmain.c:2715)
==29355==    by 0x38F6E47D87: g_main_context_iterate.isra.24 (gmain.c:3290)
==29355==    by 0x38F6E48181: g_main_loop_run (gmain.c:3484)
==29355==    by 0x40CB89: main (nmcli.c:359)
This commit is contained in:
Dan Williams 2013-03-22 15:07:17 -05:00
parent 360a02fc13
commit d57198dfa4
1 changed files with 31 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

89
libnm-glib/nm-device.c
View file

@ -1383,39 +1383,33 @@ get_decoded_property (GUdevDevice *device, const char *property)
return unescaped;
}
static void
_device_update_description (NMDevice *device)
static char *
_get_udev_property (NMDevice *device,
const char *enc_prop, /* ID_XXX_ENC */
const char *db_prop) /* ID_XXX_FROM_DATABASE */
{
NMDevicePrivate *priv;
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
const char *subsys[3] = { "net", "tty", NULL };
GUdevDevice *udev_device = NULL, *tmpdev, *olddev;
const char *ifname;
guint32 count = 0;
const char *vendor, *model;
g_return_if_fail (NM_IS_DEVICE (device));
priv = NM_DEVICE_GET_PRIVATE (device);
char *enc_value = NULL, *db_value = NULL;
if (!priv->client) {
priv->client = g_udev_client_new (subsys);
if (!priv->client)
return;
return NULL;
}
ifname = nm_device_get_iface (device);
if (!ifname)
return;
return NULL;
udev_device = g_udev_client_query_by_subsystem_and_name (priv->client, "net", ifname);
if (!udev_device)
udev_device = g_udev_client_query_by_subsystem_and_name (priv->client, "tty", ifname);
if (!udev_device)
return;
g_free (priv->product);
priv->product = NULL;
g_free (priv->vendor);
priv->vendor = NULL;
return NULL;
/* Walk up the chain of the device and its parents a few steps to grab
* vendor and device ID information off it.
@ -1425,43 +1419,11 @@ _device_update_description (NMDevice *device)
* as g_udev_device_get_parent() returns a ref-ed object.
*/
tmpdev = g_object_ref (udev_device);
while ((count++ < 3) && tmpdev && (!priv->vendor || !priv->product)) {
if (!priv->vendor)
priv->vendor = get_decoded_property (tmpdev, "ID_VENDOR_ENC");
if (!priv->product)
priv->product = get_decoded_property (tmpdev, "ID_MODEL_ENC");
olddev = tmpdev;
tmpdev = g_udev_device_get_parent (tmpdev);
g_object_unref (olddev);
}
/* Unref the last device if we found what we needed before running out
* of parents.
*/
if (tmpdev)
g_object_unref (tmpdev);
/* If we didn't get strings directly from the device, try database strings */
/* Again, ref the original device as we need to unref it every iteration
* since g_udev_device_get_parent() returns a refed object.
*/
tmpdev = g_object_ref (udev_device);
count = 0;
while ((count++ < 3) && tmpdev && (!priv->vendor || !priv->product)) {
if (!priv->vendor) {
vendor = g_udev_device_get_property (tmpdev, "ID_VENDOR_FROM_DATABASE");
if (vendor)
priv->vendor = g_strdup (vendor);
}
if (!priv->product) {
model = g_udev_device_get_property (tmpdev, "ID_MODEL_FROM_DATABASE");
if (model)
priv->product = g_strdup (model);
}
while ((count++ < 3) && tmpdev && !enc_value) {
if (!enc_value)
enc_value = get_decoded_property (tmpdev, enc_prop);
if (!db_value)
db_value = g_strdup (g_udev_device_get_property (tmpdev, db_prop));
olddev = tmpdev;
tmpdev = g_udev_device_get_parent (tmpdev);
@ -1477,8 +1439,15 @@ _device_update_description (NMDevice *device)
/* Balance the initial g_udev_client_query_by_subsystem_and_name() */
g_object_unref (udev_device);
_nm_object_queue_notify (NM_OBJECT (device), NM_DEVICE_VENDOR);
_nm_object_queue_notify (NM_OBJECT (device), NM_DEVICE_PRODUCT);
/* Prefer the the encoded value which comes directly from the device
* over the hwdata database value.
*/
if (enc_value) {
g_free (db_value);
return enc_value;
}
return db_value;
}
/**
@ -1498,8 +1467,10 @@ nm_device_get_product (NMDevice *device)
g_return_val_if_fail (NM_IS_DEVICE (device), NULL);
priv = NM_DEVICE_GET_PRIVATE (device);
if (!priv->product)
_device_update_description (device);
if (!priv->product) {
priv->product = _get_udev_property (device, "ID_MODEL_ENC", "ID_PRODUCT_FROM_DATABASE");
_nm_object_queue_notify (NM_OBJECT (device), NM_DEVICE_PRODUCT);
}
return priv->product;
}
@ -1520,8 +1491,10 @@ nm_device_get_vendor (NMDevice *device)
g_return_val_if_fail (NM_IS_DEVICE (device), NULL);
priv = NM_DEVICE_GET_PRIVATE (device);
if (!priv->vendor)
_device_update_description (device);
if (!priv->vendor) {
priv->vendor = _get_udev_property (device, "ID_VENDOR_ENC", "ID_VENDOR_FROM_DATABASE");
_nm_object_queue_notify (NM_OBJECT (device), NM_DEVICE_VENDOR);
}
return priv->vendor;
}