- ensure, that dispatcher_results_process() logs a line even if no scripts
were run. This way we alyways know when the callout returns.
- log a line when cancelling a dispatcher call
Signed-off-by: Thomas Haller <thaller@redhat.com>
When delaying the deactivation of a device during dispatcher-pre-down,
we must preseve the reason to pass it on.
This is especially important, because nm_device_slave_notify_release()
checks for the reason, and does not deactivate the slave if no reason is
given. This error caused slaves the be left up when deactivating the master.
Also update the call to nm_device_slave_notify_release() to ensure we
have a valid state reason when configuring the slave. This would have
pointed out the issue and would even work around it.
Regression introduced by commit d00e2147de.
Signed-off-by: Thomas Haller <thaller@redhat.com>
NM fails to activate a slave if the master device already exists
but has not active connection.
One way to reproduce, create a bond master/slave configuration and
ensure that the master device exists (e.g. by activating the bond, and
killing NM without taking down the device, or externally via `ip link add`).
If you try to activate the slave it will fail with the following message
(in nmcli):
"Error: Connection activation failed: The active connection on MASTER is not a valid master for 'SLAVE'"
although MASTER is not active.
This also triggers the following assertion:
#0 0x0000003370c504e9 in g_logv () from /lib64/libglib-2.0.so.0
#1 0x0000003370c5063f in g_log () from /lib64/libglib-2.0.so.0
#2 0x000000000047646a in is_compatible_with_slave (master=0x0, slave=slave@entry=0xc4aa60) at nm-manager.c:2193
#3 0x000000000047e289 in ensure_master_active_connection (self=self@entry=0xc8d150, subject=0x7f23b80059e0, connection=connection@entry=0xc4aa60, device=device@entry=0xcac380, master_connection=master_connection@entry=0x0,
master_device=master_device@entry=0xc9e800, error=error@entry=0x7fffa5cc4958) at nm-manager.c:2395
#4 0x000000000047eb4a in _internal_activate_device (self=self@entry=0xc8d150, active=active@entry=0xcc33b0, error=error@entry=0x7fffa5cc4958) at nm-manager.c:2665
#5 0x000000000047ecf2 in _internal_activate_generic (self=self@entry=0xc8d150, active=active@entry=0xcc33b0, error=error@entry=0x7fffa5cc4958) at nm-manager.c:2712
#6 0x000000000047ef2b in _internal_activation_auth_done (active=0xcc33b0, success=<optimized out>, error_desc=0x0, user_data1=0xc8d150, user_data2=<optimized out>) at nm-manager.c:2848
#7 0x0000000000466fa1 in auth_done (chain=0xcef020, error=0x0, unused=<optimized out>, user_data=<optimized out>) at nm-active-connection.c:603
#8 0x00000000004753da in auth_chain_finish (user_data=0xcef020) at nm-manager-auth.c:88
#9 0x0000003370c492a6 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0
#10 0x0000003370c49628 in g_main_context_iterate.isra () from /lib64/libglib-2.0.so.0
#11 0x0000003370c49a3a in g_main_loop_run () from /lib64/libglib-2.0.so.0
#12 0x0000000000429e65 in main (argc=1, argv=0x7fffa5cc4e48) at main.c:678
Signed-off-by: Thomas Haller <thaller@redhat.com>
Coverity gets confused and thinks we are potentially leaking bssid_str
here. Given that nm_utils_hwaddr_ntoa() never returns NULL anyway,
just drop the check.
(cherry picked from commit f4f4ecf159)
The G_DISABLE_CHECKS version of g_return_if_reached() still returns,
it just doesn't log. So don't include a manual return after a
return-if-reached.
(cherry picked from commit 3c13d9e3fd)
If a valid connection was updated and still valid, and then was
updated and become invalid, the connection would not be properly
removed from the ifnet plugin's priv->connections hash, and thus
would never be disposed.
This was due to using the direct pointer to the connection's UUID
as the key for the hash table. When a connection is updated and
its settings are replaced, the old UUID is freed and replaced with
a new pointer. But the ifnet plugin hash table still uses the
old (now freed) UUID pointer as the key. Thus when the connection
is updated and becomes invalid, looking up the UUID in the hash
table fails to find the connection, and the connection is not
removed from the hash.
This bug could cause a crash in some cases, if two keys of the
GHashTable hashed to the same value, in which case GLib would
call g_str_equal() on the freed pointer.
Since code other than in the ifnet plugin replaces settings,
we cannot be guaranteed that the pointer won't change. Avoid all
that and just strdup() the UUID when using it as a key.
Since the pointer to the connection's path could change any time
commit_changes() is called, it's not safe to use it as the hash
table key directly. strdup it instead.
Prevents:
Connection failed to verify: (unknown)
invalid or missing connection property 'blah blah/foo bar'
Simply removing the warning in reader.c is fine, because callers that
care already log the warning themselves. Also make the warning in
update_connection() the same as the warning in new_connection().
If a valid connection was updated and still valid, and then was
updated and become invalid, the connection would not be properly
removed from the keyfile plugin's priv->connections hash, and thus
would never be disposed.
This was due to using the direct pointer to the connection's UUID
as the key for the hash table. When a connection is updated and
its settings are replaced, the old UUID is freed and replaced with
a new pointer. But the keyfile plugin hash table still uses the
old (now freed) UUID pointer as the key. Thus when the connection
is updated and becomes invalid, looking up the UUID in the hash
table fails to find the connection, and the connection is not
removed from the hash.
This bug could cause a crash in some cases, if two keys of the
GHashTable hashed to the same value, in which case GLib would
call g_str_equal() on the freed pointer.
Since code other than in the keyfile plugin replaces settings,
we cannot be guaranteed that the pointer won't change. Avoid all
that and just strdup() the UUID when using it as a key.
(also collapses _internal_new_connection() into its only caller)
When connected to a phone via bluetooth and turning bluetooth off on the
computer NetworkManegr crashed due to accessing invalid device.
Reproducer:
- activate bluetooth on a computer and a phone
- pair the devices
- $ nmcli con add type blue con-name phone bt-type panu addr 00:17:EA:84:E7:41
- turn off bluetooth on computer (either with a hardware or software switch)
https://bugzilla.redhat.com/show_bug.cgi?id=1059494
(cherry picked from commit 948a27257a)
Without this header Buildroot's build complains about unknown
types like GFile etc.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
(cherry picked from commit c62639d572)
_rebase_relative_time_on_now() is used both by _address_get_lifetime()/nm_platform_ip[46]_address_sync()
and the to_string() functions.
In the latter case, we want to print the original value, without padding. Otherwise in
the addresses are printed in the logs with an additional 5 seconds
padding, which is confusing.
For adding addresses in platform however, we still want to keep the
padding. So pass it on as additional parameter.
Signed-off-by: Thomas Haller <thaller@redhat.com>
(cherry picked from commit 63ef089f69)
This error was introduced only recently with commit
8310a039d8.
Signed-off-by: Thomas Haller <thaller@redhat.com>
(cherry picked from commit 8366d7cd1b)
Something changed at some point so that NMManager was now recomputing
its state after a connection was activated, but before NMPolicy had
decided whether to give that connection the default route, meaning
NMManager would set the state to CONNECTED_LOCAL rather than
CONNECTED_GLOBAL.
Fix this by watching the active connection :default and :default6
properties too, so we do the right thing regardless of what order the
AC properties change in.
To ensure that NetworkManager does not block needlessly for events
which have no scripts, require scripts that respond to blocking
events to opt into the action.
Since the event loop isn't running on quit, but we want to ensure that
scripts can fully process the DOWN event, block on scripts completing
when disconnecting the VPN when quitting.
This event runs before a connection/device is announced as
"activated" or "connected", to enable scripts to do things
before applications begin using connectivity. For example,
this could be used to manage /etc/resolv.conf outside of
NetworkManager and ensure that resolv.conf had correct
information before DNS is used.
Note that this is different than the Debian or Gentoo "pre-up"
event used in /etc/network/interfaces, as that event runs before
any L2 configuration has started. If we really need an event
like that, we'll add it later as "lower-up".
Thomas pointed out that using the address of the DispatcherInfo
structure as the dispatcher call ID could cause a mis-cancelation
if malloc re-used the same block in the future. While the code
should be correctly clearing call IDs after the callback runs
or is canceled, just use numeric IDs to avoid potential crashses.
On shutdown we can't defer the response to a callback, so we need to
use synchronous D-Bus calls. Second, sometimes we want to block on
the dispatcher response, like for pre-down.
If there are no dispatcher scripts, don't bother dispatching any
events. This saves some time configuring networking if the event
would have no effect anyway.
The NMDevice dispose() function contained some badly-duplicated logic
about when to deactivate a device on its last ref. This logic should
only run when the device is removed by the manager, since the manager
controls the device's life-cycle, and the manager knows best when to
clean up the device. But since it was tied to the device's refcount,
it could have run later than the manager wanted, or not at all.
It gets better. Dispose duplicated logic that was already done in
nm_device_cleanup(), and then *called* nm_device_cleanup() if the
device was still activated and managed. But the manager already
unmanages the device when removing it, which triggers a call to
nm_device_cleanup(), takes the device down, and resets the IPv6
sysctl properties, which dispose() duplicated too. So by the time
dispose() runs, the device should already be unmanaged if the
manager wants to deconfigure it, and most of the dispose() code
should be a no-op.
Clean all that up and remove duplicated functions. Now, the flow
should be like this:
1) manager decides to remove the device and calls remove_device()
2) if the device should be deconfigured, the manager unmanages
the device
3) the NMDevice state change handler tears down the active connection
via nm_device_cleanup() and resets IPv6 sysctl properties
4) when the device's last reference is finally released, only internal
data members are freed in dispose() because the device should
already have been cleaned up by the manager and be unmanaged
5) if the device should be left running because it has an assumable
connection, then the device is not unmanaged, and no cleanup
happens in the state change handler or in dispose()
Ensure autoip4 is cleaned up when disposing, like we clean up DHCP.
Move things that only free stuff to finalize(), and move things that
unref stuff to dispose (eg, the Firewall Manager stuff).
The following procedure leaves an NMActiveConnection around for a deactivated
device, which causes errors in libnm-glib clients when they cannot create the
GObject for the non-existent device of the AC.
1) allow a device which can assume connections to be activated
2) stop NM, which should leave the device's IP configuration up
3) start NM and allow it to assume the device's existing connection
4) remove the device, either by unplugging it or 'rmmod'
The device is removed by nm-manager.c::remove_device(), but the device object
is not moved to UNMANAGED state, leaving the NMActiveConnection completely
unaware the device has gone away.
The nm-manager.c::remove_device() code did not correctly handle moving a
forcibly removed (eg, by unplugging or 'ip link del' or 'rmmod') device to
the UNMANAGED state when the device was active with an assumed connection.
To fix this, make the conditions when the device should be deactivated
on removal much more explicit.
A device should be deactivated on removal if:
1) it is forcibly removed, eg by the kernel network interface being
removed due to 'ip link del' or hotplugging, or internally by NM due
to a parent WWAN interface taking priority over a WWAN ethernet interface
2) if the device cannot assume connections, in which case NetworkManager
must have activated the device and since we cannot assume the connection
on restart, we should deactivate it
3) if the device is not activated, to ensure that its IPv6 parameters
and other things get reset to the pre-NetworkManager values
https://bugzilla.gnome.org/show_bug.cgi?id=729833
dhcp6_cleanup() frees priv->dhcp6_ip6_config so dispose() doesn't need
to do that. Also use g_clear_object() when appropriate.
Lastly, notify that the DHCP4/6 config objects have changed *after*
clearing them, so that the PropertiesChanged signal is emitted with a
blank path to indicate the object is gone. Previously the PC signal
would have been emitted with the valid path of the DHCP4/6 config object,
but the object would already be dead.
For any function in nm-device.h which is not used outside of
nm-device.c, remove the public prototypes. Functions that
are actually used get moved above their caller, and functions
that have no callers are removed.
Move the GObject-related stuff to the bottom of the file and get
rid of no-longer-necessary forward prototypes since the stuff that
required those prototypes is now below the implementation.
