Enums can also be negative (contrary to Flags). Fix the parsing.
$ nmcli connection modify "$PROFILE" connection.llmnr -1
Error: failed to modify connection.llmnr: invalid option '-1', use one of [default,no,resolve,yes].
In the vast majority of cases is the string for _nm_utils_enum_from_str_full()
short. As we duplicate it for stripping, prefer to clone it on the stack
with nm_strdup_maybe_a().
The BOOTIF MAC address can be prefixed with a hardware address
type. Typically it is 01 (for ethernet), but the legacy network module
accepts (and strips) any byte value.
It seems wrong to take any address type without validation. In
addition to "01", also accept a zero type which, according to the
bugzilla below, is used in some configurations to mean "undefined".
While at it, also accept ':' as separator for the first byte.
https://bugzilla.redhat.com/show_bug.cgi?id=1904099https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/713
This assert sometimes fails during copr builds. But the way
the assert was, it was hard to see what the actual problem
was.
Restructure the assert (again) to get the errno in the
test logs.
RFCs actually expect to honor the lifetime. See for example [1].
This is just not right, and totally arbitrary. It was added
when our libndp based implementation was added, but unclear
why this was done (beyond the code comment).
[1] page 204, v6LC.2.2.25: Processing Router Advertisement DNS (Host
only) at https://ipv6ready.org/docs/Core_Conformance_5_0_0.pdf
There is no actual change in behavior, because "struct nd_opt_hdr"
as two uint8_t, so in practice this struct was always packed already.
But make it explicit, because it's clear that we use these structs
to set the binary message and they need a well defined (packed) memory
layout.
The endpoints of WireGuard peers can be configured as DNS name, which
NetworkManager will resolve.
Since activating a profile might affect now names get resolved, we must
first resolve names before completing the activation of the WireGuard
device (and before reconfiguring DNS accordingly).
For example, if you configure exclusive DNS resolution via the WireGuard
device, and if the peer needs to be resolved via DNS, then resolving the
peer name must happen before the reconfiguration of DNS. Otherwise the
new DNS configuration will be broken due to being unable to reach the
WireGuard peer.
Fix that by waiting.
There is still an unfixed problem. If resolving any peers fails,
activation silently proceeds -- again possibly breaking the network
setup. Of course, NetworkManager will repeatedly try to re-resolve
the name, but that may never succeed if DNS would be resolved via
the VPN itself.
That is different from `wg set` which resolves hostnames and fails.
Consequently `wg-quick up` would also fail. But these are both one shot
applications, they are not around and basically let the user handle the
error (by reading the log and invoking the command again). NetworkManager
can do something different and proceed activation (as it will also
periodically re-resolve the hostnames again). Note that it's also valid
to activate a WireGuard device without any peers (and to modify the
activated device later with Reapply()). As such, having no peers (or
being unable to resolve a hostname) may be a valid configuration.
I think we should add an option/flag that when enabled will cause
the activation to fail of names cannot be resolved.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/535https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/616https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/721
With LTO and gcc-10.2.1-9.fc33.s390x we get:
src/platform/nm-platform.c:3325:1: error: link_duplex may be used uninitialized in this function [-Werror=maybe-uninitialized]
3325 | NM_UTILS_LOOKUP_STR_DEFINE(nm_platform_link_duplex_type_to_string,
| ^
src/devices/nm-device-ethernet.c:899: note: link_duplex was declared here
899 | NMPlatformLinkDuplexType link_duplex;
|
With gcc-10.2.1-9.fc33.s390x we get a (false positive) warning:
src/platform/tests/test-common.c: In function nmtstp_acd_defender_new:
src/platform/tests/test-common.c:2688:15: error: probe may be used uninitialized in this function [-Werror=maybe-uninitialized]
2688 | *defender = (NMTstpAcdDefender){
| ^
src/platform/tests/test-common.c:2656:56: note: probe was declared here
2656 | NAcdProbe * probe;
| ^
- accept directory names in the command line. In that case,
still honor the excluded files. That is a major improvement
for me, because I usually only want to reformat a directory
that I know has changed and it is fast to only process some
directories.
- pass all files at once to clang-format. For me that gives
a significant speed improvement (about 3 times faster), although
clang-format is only single threaded. Possibly clang-format could
even be faster by checking files in parallel.
In case of a style error, the script still falls back to
iterate over all files to find the first bad file and print
the full diff. But that is considered an unusual case.
- make it correctly work from calling it from a subdirectory.
In that case, we only check files inside that directory --
but still correctly honor the excluded files.
With LTO, the compiler can see that some code paths return without
initializing the variable. But it fails to see that those are code
paths after an assertion fail. Still that can lead to
"-Wmaybe-uninitialized" warnings in the caller.
Avoid that by not using g_return_if_fail() but nm_assert().
src/nm-ip6-config.c: In function '_nmtst_ip6_config_get_address':
./shared/nm-glib-aux/nm-dedup-multi.h:337:8: error: 'iter._next' may be used uninitialized in this function [-Werror=maybe-uninitialized]
337 | if (!iter->_next)
| ^
src/nm-ip6-config.c:1622:33: note: 'iter._next' was declared here
1622 | NMDedupMultiIter iter;
| ^
./shared/nm-glib-aux/nm-dedup-multi.h:343:8: error: 'iter._head' may be used uninitialized in this function [-Werror=maybe-uninitialized]
343 | if (iter->_next->next == iter->_head)
| ^
src/nm-ip6-config.c:1622:33: note: 'iter._head' was declared here
1622 | NMDedupMultiIter iter;
| ^
and more.
When compiling with LTO, the compiler can think that an
assertion failure (g_return*()) is regular code path, and
thus that the output variable is not set.
This can lead to "-Wmaybe-uninitialized" warnings in the
caller, despite this not happening in non-bug case.
Work aound that by setting the out argument.
Warning with LTO enabled and gcc-10.2.1-9.fc33.s390x:
src/nm-config-data.c: In function nm_config_data_get_device_config:
src/devices/nm-device.c:17454:9: error: is_fake may be used uninitialized in this function [-Werror=maybe-uninitialized]
17454 | m = nm_match_spec_device(specs,
| ^
src/devices/nm-device.c:17444:26: note: is_fake was declared here
17444 | gboolean is_fake;
| ^
src/nm-config-data.c: In function nm_config_data_get_connection_default:
src/devices/nm-device.c:17454:9: error: is_fake may be used uninitialized in this function [-Werror=maybe-uninitialized]
17454 | m = nm_match_spec_device(specs,
| ^
src/devices/nm-device.c:17444:26: note: is_fake was declared here
17444 | gboolean is_fake;
| ^
src/devices/nm-device.c: In function nm_device_check_unrealized_device_managed:
src/devices/nm-device.c:17454:9: error: is_fake may be used uninitialized in this function [-Werror=maybe-uninitialized]
17454 | m = nm_match_spec_device(specs,
| ^
src/devices/nm-lldp-listener.c: In function 'lldp_neighbor_to_variant':
./shared/nm-glib-aux/nm-shared-utils.h:1271:5: error: 'raw_len' may be used uninitialized in this function [-Werror=maybe-uninitialized]
1271 | g_variant_builder_add(builder,
| ^
src/devices/nm-lldp-listener.c:107:19: note: 'raw_len' was declared here
107 | gsize raw_len;
| ^
./shared/nm-glib-aux/nm-shared-utils.h:1271:5: error: 'raw_data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
1271 | g_variant_builder_add(builder,
| ^
src/devices/nm-lldp-listener.c:106:19: note: 'raw_data' was declared here
106 | gconstpointer raw_data;
| ^
With LTO build on s390x (Fedora 33) we get a compiler warning:
libnm-core/nm-setting-ethtool.c: In function 'nm_setting_ethtool_get_optnames':
libnm-core/nm-setting-ethtool.c:263:60: error: 'len' may be used uninitialized in this function [-Werror=maybe-uninitialized]
263 | return len > 0 ? nm_memdup(names, sizeof(names[0]) * (((gsize) len) + 1u)) : NULL;
| ^
libnm-core/nm-setting-ethtool.c:257:24: note: 'len' was declared here
257 | guint len;
| ^
libnm-core/nm-setting-ethtool.c: In function 'nm_setting_ethtool_get_optnames':
libnm-core/nm-setting-ethtool.c:263:60: error: 'len' may be used uninitialized in this function [-Werror=maybe-uninitialized]
263 | return len > 0 ? nm_memdup(names, sizeof(names[0]) * (((gsize) len) + 1u)) : NULL;
| ^
libnm-core/nm-setting-ethtool.c:257:24: note: 'len' was declared here
257 | guint len;
| ^
The change broke unit tests on 32 bit systems.
Change the code again to make it more similar to what it was
before. Now only on 64 bit systems there is any difference compared
to before. That makes it easier about reasoning for how the unit test
should be (in most cases, it is unchanged).
Fixes: 040c86f15c ('shared: avoid compiler warning for nm_utils_get_next_realloc_size() returning huge sizes')
With LTO enabled, the compiler might think that "len" is not initialized.
That is even a correct assumption, if the compiler does not understand the
API of sendmsg() and that sendmsg() is supposed to set a negative errno.
Work around by initializing the variable.
shared/n-dhcp4/src/n-dhcp4-c-connection.c: In function n_dhcp4_c_connection_send_request:
shared/n-dhcp4/src/n-dhcp4-socket.c:368:19: error: len may be used uninitialized in this function [-Werror=maybe-uninitialized]
} else if (len != n_buf) {
^
shared/n-dhcp4/src/n-dhcp4-socket.c:351:23: note: len was declared here
size_t n_buf, len;
^
On s390x (gcc-8.3.1-5.1.el8.s390x) the compiler warns that we don't
pass size larger than 2^63-1 to malloc. With LTO enabled, it is also
quite adamant in detecting that with nm_utils_get_next_realloc_size().
Optimally, we would disable this useless warning with "-Wno-alloc-size-larger-than",
but that seems not to work. So add a workaround in code :(
It's hard to actually workaround the warning while handling all kinds of
sizes. The only simple solution is to no handle such huge cases and only
assert.
In function 'nm_secret_mem_realloc',
inlined from '_nm_str_buf_ensure_size' at shared/nm-glib-aux/nm-shared-utils.c:5316:31:
shared/nm-glib-aux/nm-secret-utils.h:180:17: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
m_new = g_malloc(new_len);
^
shared/nm-glib-aux/nm-secret-utils.h: In function '_nm_str_buf_ensure_size':
/usr/include/glib-2.0/glib/gmem.h:78:10: note: in a call to allocation function 'g_malloc' declared here
gpointer g_malloc (gsize n_bytes) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1);
^
lto1: all warnings being treated as errors
RFC4361 intends to set the same IAID/DUID for both DHCPv4 and DHCPv6.
Previously, we didn't have a mode for that.
Of course, you could always set "ipv4.dhcp-client-id" and
"ipv6.dhcp-duid" to (the same) hex string, but there was no
automatic mode. Instead we had:
- "ipv4.dhcp-client-id=duid" which sets the client ID to a stable,
generated DUID. However, there was no option so that the same
DUID/IAID would be automatically used for DHCPv6.
- there are various special values for "ipv6.dhcp-duid" which generate
a stable DUIDs. However, those values did not work for
"ipv4.dhcp-client-id".
Solve that by adding "ipv4.dhcp-client-id=ipv6-duid" which indicates to use
the DUID from DHCPv6's "ipv6.dhcp-duid" setting. As IAID it will prefer "ipv4.dhcp-iaid"
(if set), but fallback to "ipv6.dhcp-iaid".
https://tools.ietf.org/html/rfc4361https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/618https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/718
The default value is `optional' so the current check in place is
actually wrong and blocks NM from connecting to WPA3 networks.
Overall the check is not that useful as WPA3 Enterprise always
requires PMF to be 'required' so let's always force it instead.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
Fixes: e874ccc917 ('wifi: add WPA-EAP-SUITE-B-192 support')
This file has no relevance for NetworkManager. And we should not deviate
from upstream n-dhcp4 which we include via git-subtree. Revert the
change.
This partly reverts commit 7f4a7bf433.
Since "_i" is int type and G_N_ELEMENTS() is size_t, the compiler
can warn
error: comparison of unsigned expression in '< 0' is always false [-Werror=type-limits]
Avoid the warning by casting.
It's questionable whether the manual page should explain
exactly what it does.
However, it's a good exercise writing this up (to review
what happens). Also, a manual page that simply says "it configures
the network automatically" without going into how exactly, is
not very useful either.
We use the iface_idx for example to determine the routing table,
by using table 30400+iface_idx.
While the HTTP API for Azure has a index, it does not mean that we
should use that index as-is for our purpose.
Instead, treat those indexes separately and ensure that the
iface_idx that we return is numbering the interfaces starting
from zero.
While it's not clear whether we should be strict or forgiving
when fetching the HTTP meta data, we should be consistent.
On a parse error of the IP addresses we fail. Hence also
fail on a parse error for the subnet.
