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 764606 - NMManager crash in platform_link_added
NMManager crash in platform_link_added
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
1.2.x
Other All
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks: nm-review nm-1-2
 
 
Reported: 2016-04-04 17:28 UTC by Tony Espy
Modified: 2016-04-04 19:29 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
manager: refactor error variable in platform_link_added() (2.29 KB, patch)
2016-04-04 17:51 UTC, Thomas Haller 		none Details | Review
device: ensure @error always set by nm_device_factory_create_device() (3.07 KB, patch)
2016-04-04 17:51 UTC, Thomas Haller 		none Details | Review

Description Tony Espy 2016-04-04 17:28:52 UTC 
While testing NM 1.2-beta3, I reproduced the following crash on one of our devices.

NMManager's platform_link_added function logs a warning if the factory_create_device operation fails.  The log message appends error->message, however it's possible that error == NULL on return from the factory_create_device call.
Comment 1 Tony Espy 2016-04-04 17:32:38 UTC 
I just created the following pull-request with a fix:

https://github.com/NetworkManager/NetworkManager/pull/5
Comment 2 Thomas Haller 2016-04-04 17:50:57 UTC 
In most cases, when a function fails, it *must* set the @error argument.

For consistency, in case of nm_device_factory_create_device(), the function should always set @error if it returns NULL -- regardless of whether the device is to be ignored or not.
Comment 3 Thomas Haller 2016-04-04 17:51:28 UTC 
Created attachment 325366 [details] [review]
manager: refactor error variable in platform_link_added()

Free the error via gs_free_error and create a separate instances.
Comment 4 Thomas Haller 2016-04-04 17:51:33 UTC 
Created attachment 325367 [details] [review]
device: ensure @error always set by nm_device_factory_create_device()
Comment 5 Dan Williams 2016-04-04 17:58:07 UTC 
LGTM, but perhaps update the error message text to also indicate that the plugin may want the core to ignore the device.  It's not necessarily that the plugin cannot create the device, it's that the kernel interface is part of an already-existing device (like a WWAN modem or bluetooth device) and the plugin handles it internally via ip_iface, rather than having a full NMDevice.