Reported by coverity:
>>> CID 210230: Control flow issues (UNREACHABLE)
>>> This code cannot be reached: "i = 0;".
Fixes: 09e17888f7 ('libnm: add mapping functions between string and NMClientPermission enum')
(cherry picked from commit a29b13c7f1)
Reported by coverity:
>>> CID 210222: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "f" when calling
"fseek".
Fixes: ac5206aa9c ('2007-11-21')
(cherry picked from commit 581aa981c2)
Reported by coverity:
>>> CID 210228: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "dbobj" suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
(cherry picked from commit 272f19108b)
Reported by coverity:
>>> CID 210213 Uninitialized pointer read (UNINIT)
>>> Using uninitialized value iter when calling
_nm_auto_free_variant_iter
Fixes: df1d214b2e ('clients: polkit-agent: implement polkit agent without using libpolkit')
(cherry picked from commit 8cb58ef1eb)
Reported by coverity:
>>> CID 210217: (UNINIT)
>>> Using uninitialized value "identities_gvariant" when calling
"gs_local_variant_unref".
Fixes: df1d214b2e ('clients: polkit-agent: implement polkit agent without using libpolkit')
(cherry picked from commit fbccd24db6)
The manual page claimed that for "connectivitiy-change" actions, the dispatcher
scripts would get as first argument (the device name) "none". That was not done,
only for "hostname" actions.
For consistency, maybe that should be adjusted to also pass "none" for connectivity
change events. However, "none" is really an odd value, if there is no device. Passing
an empty word is IMO nicer. So stick to that behavior, despite being inconsistent.
Also fix the documentation about that.
(cherry picked from commit 0b168f7b99)
Currently any error encountered in n_dhcp4_c_connection_dispatch_io()
causes a dispatch failure and interrupts the library state
machine. The recvmsg() on the socket can fail for different reasons;
one of these is for example that the UDP request previously sent got a
ICMP port-unreachable response. This can be reproduced in the
following way:
ip netns add ns1
ip link add veth0 type veth peer name veth1
ip link set veth1 netns ns1
ip link set veth0 up
cat > dhcpd.conf <<EOF
server-identifier 172.25.0.1;
max-lease-time 120;
default-lease-time 120;
subnet 172.25.0.0 netmask 255.255.255.0 {
range 172.25.0.100 172.25.0.200;
}
EOF
ip -n ns1 link set veth1 up
ip -n ns1 address add dev veth1 172.25.0.1/24
ip netns exec ns1 iptables -A INPUT -p udp --dport 67 -j REJECT
ip netns exec ns1 dhcpd -4 -cf dhcpd.conf -pf /tmp/dhcp-server.pid
If a client is started on veth0, it is able to obtain a lease despite
the firewall rule blocking DHCP, because dhcpd uses a packet
socket. Then it fails during the renewal because the recvmsg() fails:
dhcp4 (veth0): send REQUEST of 172.25.0.178 to 172.25.0.1
dhcp4 (veth0): error -111 dispatching events
dhcp4 (veth0): state changed bound -> fail
The client should consider such errors non fatal and keep running.
https://bugzilla.redhat.com/show_bug.cgi?id=1829178https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/486
(cherry picked from commit c5d1d4c498)
It is very uncommon that a user provides explicit SSIDs to scan.
So, most of the time there is nothing to do here.
(cherry picked from commit d9740d108d)
Only _scan_request_ssids_track() adds elements to the list, and that already
trims the list to a maxium length. In all other cases, we never expect a need
to trim the list.
(cherry picked from commit 3af9209d47)
We make decisions based on the timestamp. We should only fetch the timestamp
once, and make consistent decisions about that. Don't read different timestamps.
(cherry picked from commit a0e115cb44)
While we are not activated, there is less need to rate limit the scan
requests to 8 seconds. Only rate limit the requests for 1.5 seconds
in that case.
Also, when changing the MAC address, supplicant flushes the AP list.
We should be able to scan right away. Reset the counters for the rate
limiting and periodic scanning.
(cherry picked from commit 12a54a44f8)
As far as NMSupplicantInterface is concerned, don't clamp the
max-scan-ssids to 5. We should track the real value that wpa_supplicant
announces, and it's up to the caller to provide fewer SSIDs.
In particular, we want to limit the number of hidden SSIDs that we
accept from connection profiles, but we don't want to limit the number
of active scans via `nmcli device wifi rescan ssid $SSID [...]`.
(cherry picked from commit c9ae23af5e)
Handling the scanning is complicated.
- we want to have periodic scans. But only at certain times,
and with an increasing back off timeout.
- the user can initiate explicit scans via D-Bus. Thereby a list
of SSIDs scan be provided.
- if there are any hidden Wi-Fi profiles configured, we want
to explicitly scan for their SSIDs.
- explicit scans are not possible at any time. But we should not reject
the scan request, but instead remember to scan later, when possible.
This is a heavy rework. It also aims to fix issues of scanning since
the recent rework of supplicant handling in commit b83f07916a
('supplicant: large rework of wpa_supplicant handling') that can render
Wi-Fi scanning broken.
Fixes: b83f07916a ('supplicant: large rework of wpa_supplicant handling'):
(cherry picked from commit e07fc217ec)
We commonly use already seconds and milliseconds scales for computing timeouts.
Reduce the number of difference scales and don't also use minutes.
(cherry picked from commit f6e438860b)
GObject signals only complicate the code and are less efficient.
Also, NM_DEVICE_AUTH_REQUEST signal really invoked an asynchronous
request. Of course, fundamentally emitting a signal *is* the same as
calling a method. However, implementing this as signal is really not
nice nor best practice. For one, there is a (negligible) overhead emitting
a GObject signal. But what is worse, GObject signals are not as strongly
typed and make it harder to understand what happens.
The signal had the appearance of providing some special decoupling of
NMDevice and NMManager. Of course, in practice, they were not more
decoupled (both forms are the same in nature), but it was harder to
understand how they work together.
Add and call a method nm_manager_device_auth_request() instead. This
has the notion of invoking an asynchronous method. Also, never invoke
the callback synchronously and provide a cancellable. Like every asynchronous
operation, it *must* be cancellable, and callers should make sure to
provide a mechanism to abort.
(cherry picked from commit b50702775f)
It's about as complicated to track a CList as it is to track
an allocated array. The latter requires fewer allocations and
has better locality. That makes it preferable.
(cherry picked from commit d935692bc7)
We want that our asynchronous operations are cancellable.
In fact, NMAuthChain is already (manually) cancellable by the
user calling nm_auth_chain_destroy(). However, sometimes we have a
GCancellable at hand, so the callers would have to register to the
cancellable themselves.
Instead, support setting a cancellable to the NMAuthChain, that aborts
the request and invokes the callback.
It does so always on an idle handler. Also, the user may only set the
cancellable once, and only before starting the first call.
(cherry picked from commit ef7fd9e4e3)
NMDevice already has access to the NMSettings singleton. It is permissible that
NMDevice *knows* about NMManager. The current alternative is emitting GObject signals
like NM_DEVICE_AUTH_REQUEST, pretending that NMDevice and NMManager would be completely
independent, or that there could be anybody else handling the request aside NMManager.
No, NMManager and NMDevice may know each other and refer to each other. Just like
NMDevice also knows and refers to NMSettings.
(cherry picked from commit 800ac28cca)
When handling a GCancellable, you make decisions based on when the cancelled
property of a GCancellable changes. Correctly handling a cancellable becoming
uncancelled again is really complicated, nor is it clear what it even means:
should the flipping be treated as cancellation or not? Probably if the
cancelled property gets reset, you already start aborting and there is
no way back. So, you would want that a cancellation is always handled.
But it's hard to implement that correctly, and it's odd to claim
something was cancelled, if g_cancellable_is_cancelled() doesn't agree
(anymore).
Avoid such problems by preventing users to call g_cancellable_reset().
(cherry picked from commit ee7fbc954e)
The Station.ConnectHiddenNetwork will provision a network in the iwd
known-networks list. This should allow us to later use the
Network.Connect interface to connect in the future.
(Note: Attempts to use Station.ConnectHiddenNetwork on already provisioned
networks, i.e. networks iwd knows about, will fail.)
This commit squashed several fixups made by thaller.
(cherry picked from commit 69aeed4bdc)
Newer versions of iwd has supported connecting to hidden networks for a
while now. There's a separate "connect-hidden" command in iwctl that
needs to be used instead of the regular "connect" command.
The equivalent on dbus is to use ConnectHiddenNetwork instead of
Connect on the Station interface. NetworkManager however uses the
Network interface and given we the explicit SSID usage we can connect
to hidden networks with that.
This change disabled the explicit check that disallows even attempting
hidden networks when using iwd.
This has been tested to work with a previously known hidden network.
Tests connecting to a previously unknown network has failed.
(cherry picked from commit cd095f49dc)
If a device only has an IPv6 link-local address, we don't generate an
assumed connection. Therefore, when a new slave connection (without IP
configuration) is activated on the device, we don't deactivate any
existing connection and the link-local address remains configured.
The IP configuration of an activated slave should be predictable and
not depend on the previous state; let's flush addresses and routes on
activation.
https://bugzilla.redhat.com/show_bug.cgi?id=1816517https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/480
(cherry picked from commit e302f5ff77)
I find it simpler to follow the pattern of checking conditions and
"erroring out", by going to the next entry. The entire loop already
behaves like that.
(cherry picked from commit f89b841b37)
