This improves performance of fuzzer.
C.f. oss-fuzz#11019.
(cherry picked from commit 3c72b6ed4252e7ff5f7704bfe44557ec197b47fa)
(cherry picked from commit 50403cccee)
(cherry picked from commit f11f5abb1a)
An assertion in dhcp_network_bind_raw_socket() is triggered when
starting an sd_dhcp_client without setting setting a MAC address
first.
- sd_dhcp_client_start()
- client_start()
- client_start_delayed()
- dhcp_network_bind_raw_socket()
In that case, the arp-type and MAC address is still unset. Note that
dhcp_network_bind_raw_socket() already checks for a valid arp-type
and MAC address below, so we should just gracefully return -EINVAL.
Maybe sd_dhcp_client_start() should fail earlier when starting without
MAC address. But the failure here will be correctly propagated and
the start aborted.
See-also: https://github.com/systemd/systemd/pull/10054
(cherry picked from commit 34af574d58)
(cherry picked from commit 0a797bdc2a)
We must disconnect ActivateInfo before invoking callbacks.
Otherwise, it can happen that the callee cancels the cancellable,
which in turn enters activate_info_complete() again, and leads
to a crash.
https://bugzilla.redhat.com/show_bug.cgi?id=1642625
(cherry picked from commit ec37e18c64)
(cherry picked from commit 2c6fafad7a)
When a software device is removed by nmcli in parallel with a
disconnection, e.g.:
nmcli connection add type team ifname t1 con-name t1
sleep 1
nmcli connection down t1 & nmcli device delete t1
nmcli sometimes crashes in the following way:
...
Connection 't1' (e4701688-d1a9-4942-85f0-a2081e120023) successfully added.
Connection 't1' successfully deactivated (D-Bus active path: /org/freedesktop/NetworkManager/ActiveConnection/36)
Device 't1' successfully removed.
AddressSanitizer:DEADLYSIGNAL
=================================================================
==15217==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000000b (pc 0x7fa6d92d1c9d bp 0x0000004ba260 sp 0x7ffffe6a6f40 T0)
==15217==The signal is caused by a READ memory access.
==15217==Hint: address points to the zero page.
0 0x7fa6d92d1c9c in g_string_truncate (/lib64/libglib-2.0.so.0+0x6ec9c)
1 0x7fa6d92d2d7b in g_string_printf (/lib64/libglib-2.0.so.0+0x6fd7b)
2 0x45a6d7 in delete_device_cb clients/cli/devices.c:2465
3 0x7fa6d9849289 in g_simple_async_result_complete /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gsimpleasyncresult.c:802
4 0x7fa6dbaa9836 in device_delete_cb libnm/nm-device.c:2458
5 0x7fa6d985bcf3 in g_task_return_now /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1148
6 0x7fa6d985c7a5 in g_task_return /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1206
7 0x7fa6d989ca6c in reply_cb /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gdbusproxy.c:2586
8 0x7fa6d985bcf3 in g_task_return_now /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1148
9 0x7fa6d985c7a5 in g_task_return /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1206
10 0x7fa6d98913c0 in g_dbus_connection_call_done /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gdbusconnection.c:5722
11 0x7fa6d985bcf3 in g_task_return_now /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1148
12 0x7fa6d985bd2c in complete_in_idle_cb /usr/src/debug/glib2-2.56.1-1.fc28.x86_64/gio/gtask.c:1162
13 0x7fa6d92ac0ea in g_idle_dispatch gmain.c:5535
14 0x7fa6d92af7cc in g_main_dispatch gmain.c:3177
15 0x7fa6d92afb97 in g_main_context_iterate gmain.c:3903
16 0x7fa6d92afec1 in g_main_loop_run (/lib64/libglib-2.0.so.0+0x4cec1)
17 0x472892 in main clients/cli/nmcli.c:1067
18 0x7fa6d8cc31ba in __libc_start_main (/lib64/libc.so.6+0x231ba)
19 0x4162b9 in _start (/usr/bin/nmcli+0x4162b9)
The reason is that after calling nm_device_delete_async() we also
listen for the manager device-removed signal. When the signal is
received, device_removed_cb() destroy the @info structure and calls
g_main_loop_quit (loop). However, if the delete_device_cb() callback
has already been dispatched it is executed anyway and it tries to
access a stale @info.
It makes little sense to listen for the device-removed signal since
the return value of nm_device_delete_async() already tells us whether
the device was removed successfully or not.
The only advantage would be that when the device goes away for other
reasons we can still return success, but that is racy and should not
be relied upon.
https://bugzilla.redhat.com/show_bug.cgi?id=1639208
(cherry picked from commit 6130a4561e)
(cherry picked from commit 8123c42e61)
I am not sure, we ever call complete_address() for router-configurations.
Maybe not, so the dad-counter is never incremented and does not matter either.
If we however do, then we certainly want to preserve the DAD counter
when the address is already tracked.
(cherry picked from commit 8c6629b356)
(cherry picked from commit 036d1f56ea)
we use get_expiry() to compare two lifetimes. Note, that previously,
it would correctly truncate the calculated expiry at G_MAXINT32-1.
However, that means, that two different lifetimes that both lie
more than 68 years in the future would compare equal.
Fix that, but extending the range to int64, so that no overflow
can happen.
(cherry picked from commit b086535cb7)
(cherry picked from commit fe60843232)
No change in behavior. Just don't do so much work inside
the deeper nesting of the loop.
(cherry picked from commit 9d0a138ef0)
(cherry picked from commit 3cecb4d018)
RFC4862 5.5.3, points d) and e) make it clear, that the list of
addresses should be compared based on the prefix.
d) If the prefix advertised is not equal to the prefix of an
address configured by stateless autoconfiguration already in the
list of addresses associated with the interface (where "equal"
means the two prefix lengths are the same and the first prefix-
length bits of the prefixes are identical), and if the Valid
Lifetime is not 0, form an address (and add it to the list) by
combining the advertised prefix with an interface identifier of
the link as follows:
That means, we should not initialize the interface identifier first
(via complete_address()) and then search for the full address.
See-also: https://tools.ietf.org/search/rfc4862#section-5.5.3
(cherry picked from commit 23c417854a)
(cherry picked from commit ac5669633c)
Later, nm_ndisc_add_address() asserts that the address is not an
unspecified address. Skip it, just to be sure.
(cherry picked from commit 700b04d0de)
(cherry picked from commit e0e698e463)
Previously, we would coerce the value so that preferred is the same
as lifetime. However, RFC4862 5.5.3.c) says:
c) If the preferred lifetime is greater than the valid lifetime,
silently ignore the Prefix Information option. A node MAY wish to
log a system management error in this case.
See-also: https://tools.ietf.org/search/rfc4862#section-5.5.3
(cherry picked from commit 43c3c259c8)
(cherry picked from commit eff9e161cb)
Note how the nm_ndisc_add_*() return a boolean to indicate whether
anything changes. That is taken to decide whether to emit a changed
signal.
Previously, we would not consider all fields which are exposed
as public API.
Note that nm-ip6-config.c would care about the lifetime of NMNDiscAddress.
For that, nm_ndisc_add_address() would correctly consider a change of
the lifetime as relevant. So, this was for the most part not broken.
However, for example nm_ndisc_add_route() would ignore changes to the
gateway.
Always signal changes if anything changes at all. It's more correct
and robust.
(cherry picked from commit 98ec56c670)
(cherry picked from commit 2e12660dd4)
It should never happen that we are unable to switch the namespace.
However, in case it does, we cannot just return G_SOURCE_CONTINUE,
because we will just endlessly trying to process IO without actually
reading from the socket.
This shouldn't happen, but the instance is hosed and something is
very wrong. No longer handle the socket to avoid an endless loop.
(cherry picked from commit d444fcde34)
(cherry picked from commit 6631debaa3)
event_ready() calls ndp_callall_eventfd_handler(), which invokes
our own callback, which may invoke change notification.
At that point, it's not guaranteed that the signal handler won't
destroy the ndisc instance, which means, the "struct ndp" gets destroyed
while invoking callbacks. That's bad, because libndp is not robust
against that.
Ensure the object stays alive long enough.
(cherry picked from commit 9aa628cedb)
(cherry picked from commit efb9e2bc6b)
It's just ugly to invoke external code in the middel of an operation.
You never know, whether the handler won' unref the ndisc instance.
(cherry picked from commit 1f856b7cb3)
(cherry picked from commit a3c73e783b)
We're hooking the signal on construction, but we only queue a pending
action on reaching UNAVAILABLE state. The signal could fire in between:
<info> [1539282167.9666] manager: (msh0): new 802.11 OLPC Mesh device (/org/freedesktop/NetworkManager/Devices/4)
<info> [1539282168.1440] manager: (wlan0): new 802.11 WiFi device (/org/freedesktop/NetworkManager/Devices/5)
<info> [1539282168.1831] device (msh0): found companion WiFi device wlan0
<warn> [1539282168.2110] device (msh0): remove_pending_action (1): 'waiting-for-companion' not pending
file src/devices/nm-device.c: line 13966 (<dropped>): should not be reached
https://github.com/NetworkManager/NetworkManager/pull/229
(cherry picked from commit 08225c5e96)
(cherry picked from commit 7e793bf3b4)
See also "5df024f57a wwan: don't assume DNS info is always available"
which does the same for IPv4.
(cherry picked from commit cec7ade86c)
(cherry picked from commit 00f14736e6)
It's enough that all code paths in impl_ppp_manager_set_ifindex() log exactly
one message. Also, give all messages the same prefix, so that it's clear where
they come from.
(cherry picked from commit 2a45c32e8c)
(cherry picked from commit d3ba511cce)
In src/ppp/nm-pppd-plugin.c, it seems that pppd can invoke
phasechange(PHASE_RUNNING:) multiple times. Hence, the plugin
calls SetIfindex multiple times too. In nm-ppp-manager.c, we
want to make sure that the ifindex does not change after it
was set once. However, calling SetIfindex with the same ifindex
is not something worth warning. Just log a debug message and nothing.
Maybe the plugin should remember that it already set the ifindex,
and avoid multiple D-Bus calls. But it's unclear that that is desired.
For now, just downgrade the warning.
(cherry picked from commit 4a4439835d)
(cherry picked from commit d3e0a0f9b3)
strncpy() is deemed insecure, and it raises at least an eyebrow.
While it's save in this case, just avoid it.
(cherry picked from commit 4d11eba8c5)
(cherry picked from commit 2f6af40cd5)
libcurl does not allow removing easy-handles from within a curl
callback.
That was already partly avoided for one handle alone. That is, when
a handle completed inside a libcurl callback, it would only invoke the
callback, but not yet delete it. However, that is not enough, because
from within a callback another handle can be cancelled, leading to
the removal of (the other) handle and a crash:
==24572== at 0x40319AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24572== by 0x52DDAE5: Curl_close (url.c:392)
==24572== by 0x52EC02C: curl_easy_cleanup (easy.c:825)
==24572== by 0x5FDCD2: cb_data_free (nm-connectivity.c:215)
==24572== by 0x5FF6DE: nm_connectivity_check_cancel (nm-connectivity.c:585)
==24572== by 0x55F7F9: concheck_handle_complete (nm-device.c:2601)
==24572== by 0x574C12: concheck_cb (nm-device.c:2725)
==24572== by 0x5FD887: cb_data_invoke_callback (nm-connectivity.c:167)
==24572== by 0x5FD959: easy_header_cb (nm-connectivity.c:435)
==24572== by 0x52D73CB: chop_write (sendf.c:612)
==24572== by 0x52D73CB: Curl_client_write (sendf.c:668)
==24572== by 0x52D54ED: Curl_http_readwrite_headers (http.c:3904)
==24572== by 0x52E9EA7: readwrite_data (transfer.c:548)
==24572== by 0x52E9EA7: Curl_readwrite (transfer.c:1161)
==24572== by 0x52F4193: multi_runsingle (multi.c:1915)
==24572== by 0x52F5531: multi_socket (multi.c:2607)
==24572== by 0x52F5804: curl_multi_socket_action (multi.c:2771)
Fix that, by never invoking any callbacks when we are inside a libcurl
callback. Instead, the handle is marked for completion and queued. Later,
we complete all queue handles separately.
While at it, drop the @error argument from NMConnectivityCheckCallback.
It was only used to signal cancellation. Let's instead signal that via
status NM_CONNECTIVITY_CANCELLED.
https://bugzilla.gnome.org/show_bug.cgi?id=797136https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1792745https://bugzilla.opensuse.org/show_bug.cgi?id=1107197https://github.com/NetworkManager/NetworkManager/pull/207
Fixes: d8a31794c8
(cherry picked from commit fa40fc6d76)
(cherry picked from commit 7f05debf99)
We cannot be sure who holds a reference to the proxy, and
who is gonna call us back after the VPN connection instance
is destroyed.
(cherry picked from commit 6ebb9091d2)
(cherry picked from commit f71f9b54a8)
Got this assertion:
NetworkManager[12939]: <debug> [1536917977.4868] active-connection[0x563d8fd34540]: set state deactivated (was deactivating)
...
NetworkManager[12939]: nm-openvpn[1106] <info> openvpn[1132]: send SIGTERM
NetworkManager[12939]: nm-openvpn[1106] <info> wait for 1 openvpn processes to terminate...
NetworkManager[12939]: nm-openvpn[1106] <warn> openvpn[1132] exited with error code 1
NetworkManager[12939]: <info> [1536917977.5035] vpn-connection[0x563d8fd34540,2fdeaea3-975f-4325-8305-83ebca5eaa26,"my-openvpn-Red-Hat",0]: VPN plugin: requested secrets; state disconnected (9)
NetworkManager[12939]: plugin_interactive_secrets_required: assertion 'priv->vpn_state == STATE_CONNECT || priv->vpn_state == STATE_NEED_AUTH' failed
Meaning. We should either ensure that secrets_required_cb() signal callback
is disconnected from proxy's signal, or we gracefully handle callbacks at
unexpected moments. Do the latter.
(cherry picked from commit 92344dd084)
(cherry picked from commit 011dd919fa)
Set the execution bit on /usr/sbin/{ifup,ifdown} ghost files to match
the mode of same files installed by initscripts.
Otherwise, they will appear as changed according to rpm verify:
.M....... g /usr/sbin/ifdown
.M....... g /usr/sbin/ifup
when the alternatives mechanism is not in place.
# ll /usr/sbin/if{up,down}
-rwxr-xr-x. 1 root root 1651 Aug 24 06:23 /usr/sbin/ifdown
-rwxr-xr-x. 1 root root 5010 Aug 24 06:23 /usr/sbin/ifup
https://bugzilla.redhat.com/show_bug.cgi?id=1626517
(cherry picked from commit d8a972c575)
(cherry picked from commit 63639f338f)
