When request for getting secrets is being freed in request_free(),
cancel_callback is get_cancel_cb(). It uses parent->current as a secret agent
object. However, this object can be already freed and thus there is a problem
getting priv in nm_secret_agent_cancel_secrets:
g_return_if_fail (self != NULL);
priv = NM_SECRET_AGENT_GET_PRIVATE (self);
(gdb) p self
$66 = (NMSecretAgent *) 0x7fae9afd42e0
(gdb) p *self
$67 = {parent = {g_type_instance = {g_class = 0x0}, ref_count = 0, qdata = 0x0}}
#0 nm_secret_agent_cancel_secrets (self=0x7fae9afd42e0, call=0x1) at settings/nm-secret-agent.c:325
#1 0x00007fae9a774882 in request_free (req=0x7fae9afc48f0) at settings/nm-agent-manager.c:496
#2 0x00007fae967b251a in g_hash_table_remove_internal (hash_table=0x7fae9aefdf00, key=0x2, notify=1) at ghash.c:1276
#3 0x00007fae9a72b340 in dispose (object=0x7fae9af77200) at nm-activation-request.c:446
#4 0x00007fae96cbeee8 in g_object_unref (_object=0x7fae9af77200) at gobject.c:3160
#5 0x00007fae9a73d87c in _active_connection_cleanup (user_data=<optimized out>) at nm-manager.c:359
#6 0x00007fae967c32a6 in g_main_dispatch (context=0x7fae9aedb180) at gmain.c:3066
#7 g_main_context_dispatch (context=context@entry=0x7fae9aedb180) at gmain.c:3642
#8 0x00007fae967c3628 in g_main_context_iterate (context=0x7fae9aedb180, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3713
#9 0x00007fae967c3a3a in g_main_loop_run (loop=0x7fae9aedb860) at gmain.c:3907
So we need to ref() 'agent' when adding it to pending list, so that the object
is not freed if the secret agent unregisters and is removed.
Test case:
1. run NM and nm-applet
2. activate a Wi-Fi network
3. nm-applet will ask for a password; ignore the popup window and kill nm-applet
4. start nm-applet again
5. click the same Wi-Fi network in nm-applet
6. NM will experience problems in nm_secret_agent_cancel_secrets() or crashes
(the procedure may not be 100%, but reproduces most of the time)
https://bugzilla.redhat.com/show_bug.cgi?id=922855
GLib-CRITICAL **: g_hash_table_iter_next: assertion 'ri->version == ri->hash_table->version' failed
It is not allowed to modify hash table while it is iterated. Unfortunately,
request_remove_agent() may remove the request from the 'requests' hash table,
making it not usable in the loop hash table looping.
We need to store the request into a temporary list and call request_next_agent()
on them later (after the hash loop).
Test case:
1. start NM and nm-applet
2. activate a Wi-Fi WPA connection
3. nm-applet displays a dialog asking for a password
4. kill nm-applet
5. NetworkManager removes the nm-applet's secret agent
and runs into removing the request from hash table in the
iterating loop (via get_complete_cb)
#0 get_complete_cb (parent=0x7f3f250f2970, secrets=0x0, agent_dbus_owner=0x0, agent_username=0x0, error=0x7f3f250f7830, user_data=0x7f3f25020e10)
at settings/nm-agent-manager.c:1111
#1 0x00007f3f23b46ea5 in req_complete_error (error=0x7f3f250f7830, req=0x7f3f250f2970) at settings/nm-agent-manager.c:509
#2 request_next_agent (req=0x7f3f250f2970) at settings/nm-agent-manager.c:615
#3 0x00007f3f23b48596 in request_remove_agent (agent=0x7f3f250f4a20, req=0x7f3f250f2970) at settings/nm-agent-manager.c:631
#4 remove_agent (self=<optimized out>, owner=0x7f3f250dbff0 ":1.275") at settings/nm-agent-manager.c:130
#5 0x00007f3f23b4868d in impl_agent_manager_unregister (self=0x7f3f25020e10, context=0x7f3f250f5480) at settings/nm-agent-manager.c:374
#0 0x00007f3f1fb9c4e9 in g_logv (log_domain=0x7f3f1fbfef4e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fff156b77c0) at gmessages.c:989
#1 0x00007f3f1fb9c63f in g_log (log_domain=log_domain@entry=0x7f3f1fbfef4e "GLib", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL,
format=format@entry=0x7f3f1fc0889a "%s: assertion '%s' failed") at gmessages.c:1025
#2 0x00007f3f1fb9c679 in g_return_if_fail_warning (log_domain=log_domain@entry=0x7f3f1fbfef4e "GLib",
pretty_function=pretty_function@entry=0x7f3f1fc03c30 <__PRETTY_FUNCTION__.4571> "g_hash_table_iter_next",
expression=expression@entry=0x7f3f1fc038f0 "ri->version == ri->hash_table->version") at gmessages.c:1034
#3 0x00007f3f1fb849c0 in g_hash_table_iter_next (iter=<optimized out>, key=<optimized out>, value=<optimized out>) at ghash.c:733
#4 0x00007f3f23b484e5 in remove_agent (self=<optimized out>, owner=0x7f3f250dbff0 ":1.275") at settings/nm-agent-manager.c:129
#5 0x00007f3f23b4868d in impl_agent_manager_unregister (self=0x7f3f25020e10, context=0x7f3f250f5480) at settings/nm-agent-manager.c:374
Previously, the default wired connection was removed on quit when the
device was cleaned up. This is inconsistent with other connections.
Leave the default wired connection up when quitting to fix this
inconsistency.
This allows default wired connections to be assumed when NM starts.
Changing the default wired connection has always deleted the connection
(thus disconnecting the interface) and re-added it as a settings plugin
connection. That was always sub-optimal, but until the 'unsaved' connection
stuff landed this summer, we couldn't do anything about that. Clean
that all up, adding the connection as an unsaved connection right from
the start, which allows changes to the connection without having to
delete and recreate it, thus preventing disconnection of any interface
that is using the connection.
A new signal is added to NMSettingsConnection that is only emitted when
the connection is changed from D-Bus (thus indicating an explicit user-
requested change) since the connection may be modified internally by
NetworkManager. NM-triggered changes should not result in the connection
no longer being a default-wired connection.
https://bugzilla.gnome.org/show_bug.cgi?id=712188https://bugzilla.redhat.com/show_bug.cgi?id=1029464
These are (most likely) only warnings and not severe bugs.
Some of these changes are mostly made to get a clean run of
Coverity without any warnings.
Error found by running Coverity scan
https://bugzilla.redhat.com/show_bug.cgi?id=1025894
Co-Authored-By: Jiří Klimeš <jklimes@redhat.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
If the connection has never been saved to disk, it won't have a path yet,
but that doesn't mean we should crash. Next, when reloading connections,
only try to do connection matching on connections that have paths, otherwise
all in-memory-only connections would be removed at the end of
read_connections().
Doing so may cause NetworkManager to run into an very intensive loop in
svUnescape() in shvar.c.
This is 'top' output for very long (invalid team config) - 9309865 bytes long:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
26855 root 20 0 305m 35m 6092 R 99.8 0.9 8:08.11 NetworkManager
and still not finished.
If the connection has never been saved to disk, it won't have a path yet,
but that doesn't mean we should crash. Next, when reloading connections,
only try to do connection matching on connections that have paths, otherwise
all in-memory-only connections would be removed at the end of
read_connections().
Do some minimal verification of hostnames that come in via D-Bus, for
length and content. Otherwise we'd get as far as asking glibc to set
the system hostname, which would reject us.
rh #1025007 reports a crash on g_assert_no_error() in
connection_new_or_changed() of src/settings/plugins/ifcfg-rh/plugin.c.
From the back trace I am not 100% sure, what the problem was, but I
think that nm_settings_connection_replace_settings failed because of
nm_connection_update_secrets. Apparently such a situation can
happen and it should simply be accepted as valid.
What might have happened, is that the connection used to have
secrets (maybe it had 802.1x configured?) and then it got changed,
so update_secrets() fails because the connection no longer has a
setting to which the secrets would apply.
https://bugzilla.redhat.com/show_bug.cgi?id=1025007
Signed-off-by: Thomas Haller <thaller@redhat.com>
In Fedora, OVS ports are now identified in ifcfg files as
"TYPE=OVSPort", which NM doesn't recognize, and so it would ignore
those ifcfg files. Unfortunately, this meant that if auto-default
wasn't disabled, and there was no other configuration defined for the
device, then NM would create an NMDefaultWiredConnection for it and
screw things up.
So, add an "unrecognized-specs" settings plugin property, which allows
a plugin to indicate to NetworkManager that it knows of some
non-NetworkManager-supported connection defined for a device. This
will suppress default-wired connection creation for that device,
similar to the "no-auto-default" config file option, but determined by
the plugin instead of by manual configuration. Devices listed in
unrecognized-specs may still be managed by NetworkManager, unless they
are also listed in unmanaged-specs.
https://bugzilla.redhat.com/show_bug.cgi?id=1022256
Rather than having each connection-parsing function do its own
unmanaged-spec handling, just do it all directly from
connection_from_file(), and don't bother trying to fully parse the
file if it is unmanaged, since it won't ever be seen outside of the
plugin in that case anyway.
This also makes it possible to have an ifcfg file of an unrecognized
type be unmanaged.
We were accidentally removing the connection from priv->connections
(and thus from unmanaged-specs) when NM_CONTROLLED changed to no when
rereading a changed connection file.
If an ifcfg file changed from one non-NULL unmanaged-spec to another
(eg, if it previously had an interface-name: unmanaged-spec, and then
you add a HWADDR line, switching it to a mac: unmanaged-spec), we were
not updating the connection's unmanaged property, or emitting
unmanaged-specs-changed.
Also, remove the notify::unmanaged handler, since only plugin.c ever
changes an existing NMIfcfgConnection's unmanaged property, and it
always emits the signal itself afterward (and it needs to manually
emit the signal in other cases anyway, like when a connection is
removed).
Testcase:
* add 'NM_CONTROLLED=no' to /etc/sysconfig/network-scripts/ifcfg-ABC
* sudo nmcli con reload
* ... NM asserts ...
We need to ref() 'existing' connection before nm_settings_connection_signal_remove(),
because the function unref()s ithe connection via connection_removed_cb().
Backtrace:
...
#4 0x00007fbcf0ea0cba in g_assertion_message_expr (domain=domain@entry=0x0,
file=file@entry=0x7fbcf4e5805d "nm-dbus-manager.c", line=line@entry=848,
func=func@entry=0x7fbcf4e585e0 <__FUNCTION__.15088> "nm_dbus_manager_unregister_object", expr=expr@entry=0x7fbcf4e5820b "G_IS_OBJECT (object)")
at gtestutils.c:2293
#5 0x00007fbcf4de69d9 in nm_dbus_manager_unregister_object (
self=0x7fbcf6fdc9c0, object=0x7fbcf70235c0) at nm-dbus-manager.c:848
#6 0x00007fbcf4dd6a23 in nm_settings_connection_signal_remove (
self=<optimized out>) at settings/nm-settings-connection.c:1541
#7 0x00007fbce6fee884 in connection_new_or_changed (
self=self@entry=0x7fbcf7006f80,
path=path@entry=0x7fbcf70c3f80 "/etc/sysconfig/network-scripts/ifcfg-ABC",
existing=existing@entry=0x7fbcf70235c0,
out_old_path=out_old_path@entry=0x7fff2b7b8988) at plugin.c:327
#8 0x00007fbce6feeca2 in read_connections (plugin=0x7fbcf7006f80)
at plugin.c:453
#9 0x00007fbcf4dd8e98 in impl_settings_reload_connections (
self=0x7fbcf6fd98c0, context=0x7fbcf70bcb30) at settings/nm-settings.c:1262
...
Support new bonding options and set them carefully. The options cannot
be set arbitrarily because they interfere with each other.
This commit is forward-ported from rhel-6.5, see patch
rh901662-bond-more-options.patch, originally written by Dan Williams.
https://bugzilla.redhat.com/show_bug.cgi?id=901662https://bugzilla.redhat.com/show_bug.cgi?id=905532
Co-Authored-By: Dan Williams <dcbw@redhat.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
Static IP addresses were only read from ifcfg-* file when IP method was
'manual' (BOOTPROTO=none|static). This was to match the legacy initscripts
behaviour. However, NetworkManager supports using additional static IPs in
addition to automatically obtained (DHCP, etc.) addresses. So we now read
static IPs even for automatic methods to be able to use this feature.
https://bugzilla.redhat.com/show_bug.cgi?id=998135
When freeing one of the collections such as GArray, GPtrArray, GSList,
etc. it is common that the items inside the connections must be
freed/unrefed too.
The previous code often iterated over the collection first with
e.g. g_ptr_array_foreach and passing e.g. g_free as GFunc argument.
For one, this has the problem, that g_free has a different signature
GDestroyNotify then the expected GFunc. Moreover, this can be
simplified either by setting a clear function
(g_ptr_array_set_clear_func) or by passing the destroy function to the
free function (g_slist_free_full).
Signed-off-by: Thomas Haller <thaller@redhat.com>
nm_connection_provider_get_connections returns an internally kept
constant list to simplify handling for the users. Do not cache this
list in a static variable, instead put it in a private field.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Plugin owns the object and callers must reference it if they wish to use it outside
of the function they called "add" from. Likewise, callers of the ConnectionProvider's
add_connection method must also reference the returned object if they wish to
continue using it.
ifcfg-rh had the rule that if an ifcfg file had no BOOTPROTO and no
IPv4 addresses, then it should be treated as method=auto for
compatibility. But in fact, current ifup treats it as method=disabled,
so we should too.
https://bugzilla.gnome.org/show_bug.cgi?id=708875
Make sure that all connections returned from NMSettings or created via
AddAndActivateConnection have an NMSettingIP4Config and an
NMSettingIP6Config, with non-NULL methods, and get rid of
now-unnecessary checks for those.
Also move the slaves-can't-have-IP-config checks into the
platform-independent code as well. This also gets rid of spurious
"ignoring IP4/IP6 configuration" warnings in ifcfg-rh when reading a
slave ifcfg file.
Partly based on a patch from Pavel.
https://bugzilla.gnome.org/show_bug.cgi?id=708875
connection_from_file() requires the 'error' parameter. Not passing a
valid 'error' parameter causes the function to fail and return NULL,
which mean that commit_changes() would always re-write the connection
instead of ignoring commits where nothing has actually changed.
connection_from_file() no longer requires the unmanaged, keyfile,
or routefile parameters, so remove them.
This method returns true, if the connections are already loaded (and the
connection_loaded signal already emited).
Signed-off-by: Thomas Haller <thaller@redhat.com>
The problem is that there is only a single variable in ifcfg file holding dns
domains - DOMAIN. Thus NetworkManager writes both IPv4 and IPv6 dns-search into
it. While reading there is no way to distinguish between IPv4 and IPv6 values,
so the DOMAIN value is read and only put into IPv4 dns-search.
But, when IPv4 is disabled or invalid, the domains got lost. So in such case
we put DOMAIN variable into IPv6 instead.
https://bugzilla.redhat.com/show_bug.cgi?id=1004866
