From 0dd087e4b672125f793e2c4b164bc857c16b5298 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Jul 2019 20:56:14 +0200 Subject: [PATCH 01/74] release: bump version to 1.21.0 (development) --- configure.ac | 4 ++-- libnm-core/nm-version.h | 14 ++++++++++++++ meson.build | 2 +- shared/nm-version-macros.h.in | 1 + 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 86aac8a920..194fd0d6d8 100644 --- a/configure.ac +++ b/configure.ac @@ -7,8 +7,8 @@ dnl - add corresponding NM_VERSION_x_y_z macros in dnl "shared/nm-version-macros.h.in" dnl - update number in meson.build m4_define([nm_major_version], [1]) -m4_define([nm_minor_version], [19]) -m4_define([nm_micro_version], [90]) +m4_define([nm_minor_version], [21]) +m4_define([nm_micro_version], [0]) m4_define([nm_version], [nm_major_version.nm_minor_version.nm_micro_version]) diff --git a/libnm-core/nm-version.h b/libnm-core/nm-version.h index ee6a1e7db7..bb5b5422f6 100644 --- a/libnm-core/nm-version.h +++ b/libnm-core/nm-version.h @@ -215,4 +215,18 @@ # define NM_AVAILABLE_IN_1_20 #endif +#if NM_VERSION_MIN_REQUIRED >= NM_VERSION_1_22 +# define NM_DEPRECATED_IN_1_22 G_DEPRECATED +# define NM_DEPRECATED_IN_1_22_FOR(f) G_DEPRECATED_FOR(f) +#else +# define NM_DEPRECATED_IN_1_22 +# define NM_DEPRECATED_IN_1_22_FOR(f) +#endif + +#if NM_VERSION_MAX_ALLOWED < NM_VERSION_1_22 +# define NM_AVAILABLE_IN_1_22 G_UNAVAILABLE(1,22) +#else +# define NM_AVAILABLE_IN_1_22 +#endif + #endif /* NM_VERSION_H */ diff --git a/meson.build b/meson.build index e0e560c620..01b1d15ff2 100644 --- a/meson.build +++ b/meson.build @@ -4,7 +4,7 @@ project( # - add corresponding NM_VERSION_x_y_z macros in # "shared/nm-version-macros.h.in" # - update number in configure.ac - version: '1.19.90', + version: '1.21.0', license: 'GPL2+', default_options: [ 'buildtype=debugoptimized', diff --git a/shared/nm-version-macros.h.in b/shared/nm-version-macros.h.in index a546b1009a..eed928242e 100644 --- a/shared/nm-version-macros.h.in +++ b/shared/nm-version-macros.h.in @@ -76,6 +76,7 @@ #define NM_VERSION_1_16 (NM_ENCODE_VERSION (1, 16, 0)) #define NM_VERSION_1_18 (NM_ENCODE_VERSION (1, 18, 0)) #define NM_VERSION_1_20 (NM_ENCODE_VERSION (1, 20, 0)) +#define NM_VERSION_1_22 (NM_ENCODE_VERSION (1, 22, 0)) /* For releases, NM_API_VERSION is equal to NM_VERSION. * From 6c9880f8cacc70ed1bea3a463f29cdb171939d7b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Jul 2019 21:45:52 +0200 Subject: [PATCH 02/74] contrib/rpm: fix parsing of %real_version_major for ".0" versions --- contrib/fedora/rpm/NetworkManager.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 7c5620d251..2b0a717c55 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -40,7 +40,7 @@ %global snap %{?snapshot_dot}%{?git_sha_dot} -%global real_version_major %(printf '%s' '%{real_version}' | sed -n 's/^\\([1-9][0-9]*\\.[1-9][0-9]*\\)\\.[1-9][0-9]*$/\\1/p') +%global real_version_major %(printf '%s' '%{real_version}' | sed -n 's/^\\([1-9][0-9]*\\.[0-9][0-9]*\\)\\.[0-9][0-9]*$/\\1/p') %global systemd_units NetworkManager.service NetworkManager-wait-online.service NetworkManager-dispatcher.service From ea5e96cb06bdf2facec36ed4d6ab3f87f6ff95d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 29 Jul 2019 21:52:15 +0200 Subject: [PATCH 03/74] contrib/rpm: use --with-runstatedir=%{_rundir} instead of hard-coding /run --- contrib/fedora/rpm/NetworkManager.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 2b0a717c55..4ecf4bd4bf 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -590,7 +590,7 @@ gtkdocize autoreconf --install --force intltoolize --automake --copy --force %configure \ - --with-runstatedir=/run \ + --with-runstatedir=%{_rundir} \ --disable-silent-rules \ --disable-static \ --with-dhclient=yes \ From 98a20bfb7456998d2994d548f7a89246edc8c525 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 30 Jul 2019 17:26:53 +0200 Subject: [PATCH 04/74] libnm/doc: fix gtk-doc annotations for documenting enum values in "nm-dbus-interface.h" --- libnm-core/nm-dbus-interface.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h index 7949fa2d76..1f4b431779 100644 --- a/libnm-core/nm-dbus-interface.h +++ b/libnm-core/nm-dbus-interface.h @@ -673,7 +673,7 @@ typedef enum { * NMConnectionMultiConnect: * @NM_CONNECTION_MULTI_CONNECT_DEFAULT: indicates that the per-connection * setting is unspecified. In this case, it will fallback to the default - * value, which is @NM_CONNECTION_MULTI_CONNECT_SINGLE. + * value, which is %NM_CONNECTION_MULTI_CONNECT_SINGLE. * @NM_CONNECTION_MULTI_CONNECT_SINGLE: the connection profile can only * be active once at each moment. Activating a profile that is already active, * will first deactivate it. @@ -1045,14 +1045,14 @@ typedef enum { /*< flags >*/ * Likewise, when finally deleting the profile, both the storage from /run * and persistent storage are deleted (or if the persistent storage does not * allow deletion, and nmmeta file is written to mark the UUID as deleted). - * @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED: this is almost the same as - * @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY, with one difference: when later deleting + * @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED: this is almost the same + * as %NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY, with one difference: when later deleting * the profile, the original profile will not be deleted. Instead a nmmeta * file is written to /run to indicate that the profile is gone. * Note that if such a nmmeta tombstone file exists and hides a file in persistant * storage, then when re-adding the profile with the same UUID, then the original * storage is taken over again. - * @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY: this is like @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY, + * @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY: this is like %NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY, * but if the connection has a corresponding file on persistent storage, the file * will be deleted right away. If the profile is later again persisted to disk, * a new, unused filename will be chosen. From 2752e3a774953328da9fa8d02614cc7778a86467 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 17:54:00 +0200 Subject: [PATCH 05/74] contrib/rpm: fix libnm License tag https://bugzilla.redhat.com/show_bug.cgi?id=1723395 --- contrib/fedora/rpm/NetworkManager.spec | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 4ecf4bd4bf..a1a3cfa8a1 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -128,7 +128,7 @@ Epoch: %{epoch_version} Version: %{rpm_version} Release: %{release_version}%{?snap}%{?dist} Group: System Environment/Base -License: GPLv2+ +License: GPLv2+ and LGPLv2+ URL: http://www.gnome.org/projects/NetworkManager/ #Source: https://download.gnome.org/sources/NetworkManager/%{real_version_major}/%{name}-%{real_version}.tar.xz @@ -386,6 +386,7 @@ This package contains NetworkManager support for PPP. Summary: Libraries for adding NetworkManager support to applications (new API). Group: Development/Libraries Conflicts: NetworkManager-glib < %{epoch}:%{version}-%{release} +License: LGPLv2+ %description libnm This package contains the libraries that make it easier to use some @@ -399,6 +400,7 @@ Group: Development/Libraries Requires: %{name}-libnm%{?_isa} = %{epoch}:%{version}-%{release} Requires: glib2-devel Requires: pkgconfig +License: LGPLv2+ %description libnm-devel This package contains the header and pkg-config files for development From ce08b444710995519865503b0c12073216813ba0 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 12:36:03 +0200 Subject: [PATCH 06/74] cli: drop NMC_RETURN It's criminally ugly. Also -- totally useless. These functions return NMCResultCode, call_cmd() expects them to do so, and assigns the return_value itself. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/218 --- clients/cli/connections.c | 42 +++++++++++++++++++-------------------- clients/cli/nmcli.h | 5 ----- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 53105f957f..9a622b0b4f 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -3135,7 +3135,7 @@ do_connection_down (NmCli *nmc, int argc, char **argv) } if (arg_num == 0) { g_string_printf (nmc->return_text, _("Error: No connection specified.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } } @@ -3153,7 +3153,7 @@ do_connection_down (NmCli *nmc, int argc, char **argv) arg_ptr++; if (!arg_num) { g_string_printf (nmc->return_text, _("Error: %s argument is missing."), selector); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } } @@ -3174,7 +3174,7 @@ do_connection_down (NmCli *nmc, int argc, char **argv) if (!found_active_cons) { g_string_printf (nmc->return_text, _("Error: no active connection provided.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_NOT_FOUND); + return NMC_RESULT_ERROR_NOT_FOUND; } nm_assert (found_active_cons->len > 0); @@ -8217,7 +8217,7 @@ do_connection_edit (NmCli *nmc, int argc, char **argv) else { if (!nmc_parse_args (exp_args, TRUE, &argc, &argv, &error)) { g_string_assign (nmc->return_text, error->message); - NMC_RETURN (nmc, error->code); + return error->code; } } @@ -8247,7 +8247,7 @@ do_connection_edit (NmCli *nmc, int argc, char **argv) } else { g_string_printf (nmc->return_text, _("Error: only one of 'id', 'filename', uuid, or 'path' can be provided.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } } @@ -8261,7 +8261,7 @@ do_connection_edit (NmCli *nmc, int argc, char **argv) if (!found_con) { g_string_printf (nmc->return_text, _("Error: Unknown connection '%s'."), con); - NMC_RETURN (nmc, NMC_RESULT_ERROR_NOT_FOUND); + return NMC_RESULT_ERROR_NOT_FOUND; } /* Duplicate the connection and use that so that we need not @@ -8415,7 +8415,7 @@ do_connection_modify (NmCli *nmc, connection = get_connection (nmc, &argc, &argv, NULL, NULL, NULL, &error); if (!connection) { g_string_printf (nmc->return_text, _("Error: %s."), error->message); - NMC_RETURN (nmc, error->code); + return error->code; } rc = nm_client_get_connection_by_uuid (nmc->client, @@ -8423,12 +8423,12 @@ do_connection_modify (NmCli *nmc, if (!rc) { g_string_printf (nmc->return_text, _("Error: Unknown connection '%s'."), nm_connection_get_uuid (connection)); - NMC_RETURN (nmc, NMC_RESULT_ERROR_NOT_FOUND); + return NMC_RESULT_ERROR_NOT_FOUND; } if (!nmc_read_connection_properties (nmc, NM_CONNECTION (rc), &argc, &argv, &error)) { g_string_assign (nmc->return_text, error->message); - NMC_RETURN (nmc, error->code); + return error->code; } if (nmc->complete) @@ -8506,7 +8506,7 @@ do_connection_clone (NmCli *nmc, int argc, char **argv) connection = get_connection (nmc, argc_ptr, argv_ptr, NULL, NULL, NULL, &error); if (!connection) { g_string_printf (nmc->return_text, _("Error: %s."), error->message); - NMC_RETURN (nmc, error->code); + return error->code; } if (nmc->complete) @@ -8519,12 +8519,12 @@ do_connection_clone (NmCli *nmc, int argc, char **argv) _("New connection name: ")); } else { g_string_printf (nmc->return_text, _("Error: argument is missing.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } if (next_arg (nmc->ask ? NULL : nmc, argc_ptr, argv_ptr, NULL) == 0) { g_string_printf (nmc->return_text, _("Error: unknown extra argument: '%s'."), *argv); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } new_connection = nm_simple_connection_new_clone (connection); @@ -8855,7 +8855,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) filename = nm_strstrip (filename_ask); } else { g_string_printf (nmc->return_text, _("Error: No arguments provided.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } } @@ -8871,7 +8871,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) argv++; if (!argc) { g_string_printf (nmc->return_text, _("Error: %s argument is missing."), *(argv-1)); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } if ( argc == 1 @@ -8892,7 +8892,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) argv++; if (!argc) { g_string_printf (nmc->return_text, _("Error: %s argument is missing."), *(argv-1)); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } if (argc == 1 && nmc->complete) nmc->return_value = NMC_RESULT_COMPLETE_FILE; @@ -8902,7 +8902,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) g_printerr (_("Warning: 'file' already specified, ignoring extra one.\n")); } else { g_string_printf (nmc->return_text, _("Unknown parameter: %s"), *argv); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } next_arg (nmc, &argc, &argv, NULL); @@ -8913,11 +8913,11 @@ do_connection_import (NmCli *nmc, int argc, char **argv) if (!type) { g_string_printf (nmc->return_text, _("Error: 'type' argument is required.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } if (!filename) { g_string_printf (nmc->return_text, _("Error: 'file' argument is required.")); - NMC_RETURN (nmc, NMC_RESULT_ERROR_USER_INPUT); + return NMC_RESULT_ERROR_USER_INPUT; } if (nm_streq (type, "wireguard")) @@ -8926,7 +8926,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) service_type = nm_vpn_plugin_info_list_find_service_type (nm_vpn_get_plugin_infos (), type); if (!service_type) { g_string_printf (nmc->return_text, _("Error: failed to find VPN plugin for %s."), type); - NMC_RETURN (nmc, NMC_RESULT_ERROR_UNKNOWN); + return NMC_RESULT_ERROR_UNKNOWN; } /* Import VPN configuration */ @@ -8934,7 +8934,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) if (!plugin) { g_string_printf (nmc->return_text, _("Error: failed to load VPN plugin: %s."), error->message); - NMC_RETURN (nmc, NMC_RESULT_ERROR_UNKNOWN); + return NMC_RESULT_ERROR_UNKNOWN; } connection = nm_vpn_editor_plugin_import (plugin, filename, &error); @@ -8943,7 +8943,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) if (!connection) { g_string_printf (nmc->return_text, _("Error: failed to import '%s': %s."), filename, error->message); - NMC_RETURN (nmc, NMC_RESULT_ERROR_UNKNOWN); + return NMC_RESULT_ERROR_UNKNOWN; } add_connection (nmc->client, diff --git a/clients/cli/nmcli.h b/clients/cli/nmcli.h index cd50333a99..1c4b4a2d89 100644 --- a/clients/cli/nmcli.h +++ b/clients/cli/nmcli.h @@ -149,11 +149,6 @@ typedef struct _NmCli { char *palette_buffer; /* Buffer with sequences for terminal-colors.d(5)-based coloring. */ } NmCli; -#define NMC_RETURN(nmc, rvalue) \ - G_STMT_START { \ - return ((nmc)->return_value = (rvalue)); \ - } G_STMT_END - extern NmCli nm_cli; /* Error quark for GError domain */ From 577769b98384f7f93480a90031a407a51347aab5 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 12:23:03 +0200 Subject: [PATCH 07/74] cli: abort on extra arguments Instead of straight-out rejecting extra parameters for various nmcli sub-commands (such as "nmcli dev status", "nmcli dev wifi rescan" or "nmcli dev wifi connect", etc.), we just print a warning and go ahead. This is unhelpful. In case the user makes a typo, we'll do the wrong thing and possibly even drown the warning in the output. While at that, let's make the error message consistent. One less string to translate. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/217 --- clients/cli/connections.c | 5 +-- clients/cli/devices.c | 25 ++++++++----- .../test_003.expected | 36 +++++-------------- clients/tests/test-client.py | 2 +- 4 files changed, 28 insertions(+), 40 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 9a622b0b4f..806e3504c1 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -2933,7 +2933,8 @@ do_connection_up (NmCli *nmc, int argc, char **argv) pwds = *argv; } else if (!nmc->complete) { - g_printerr (_("Unknown parameter: %s\n"), *argv); + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); + return NMC_RESULT_ERROR_USER_INPUT; } next_arg (nmc, &argc, &argv, NULL); @@ -8901,7 +8902,7 @@ do_connection_import (NmCli *nmc, int argc, char **argv) else g_printerr (_("Warning: 'file' already specified, ignoring extra one.\n")); } else { - g_string_printf (nmc->return_text, _("Unknown parameter: %s"), *argv); + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); return NMC_RESULT_ERROR_USER_INPUT; } diff --git a/clients/cli/devices.c b/clients/cli/devices.c index c00d3191a9..b37b40e08a 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1681,9 +1681,9 @@ do_devices_status (NmCli *nmc, int argc, char **argv) if (nmc->complete) return nmc->return_value; - while (argc > 0) { - g_printerr (_("Unknown parameter: %s\n"), *argv); - next_arg (nmc, &argc, &argv, NULL); + if (argc) { + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); + return NMC_RESULT_ERROR_USER_INPUT; } if (!nmc->required_fields || strcasecmp (nmc->required_fields, "common") == 0) @@ -3027,8 +3027,10 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv) if (nmc->complete) return nmc->return_value; - if (argc) - g_printerr (_("Unknown parameter: %s\n"), *argv); + if (argc) { + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); + return NMC_RESULT_ERROR_USER_INPUT; + } if (rescan == NULL || strcmp (rescan, "auto") == 0) { rescan_cutoff = NM_MAX (nm_utils_get_timestamp_msec () - 30 * NM_UTILS_MSEC_PER_SECOND, 0); @@ -3291,7 +3293,9 @@ do_device_wifi_connect_network (NmCli *nmc, int argc, char **argv) goto finish; } } else if (!nmc->complete) { - g_printerr (_("Unknown parameter: %s\n"), *argv); + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); + nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; + goto finish; } next_arg (nmc, &argc, &argv, NULL); @@ -3784,7 +3788,7 @@ do_device_wifi_hotspot (NmCli *nmc, int argc, char **argv) } else if (nmc_arg_is_option (*argv, "show-password")) { show_password = TRUE; } else { - g_string_printf (nmc->return_text, _("Error: Unknown parameter %s."), *argv); + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); return NMC_RESULT_ERROR_USER_INPUT; } @@ -3966,8 +3970,11 @@ do_device_wifi_rescan (NmCli *nmc, int argc, char **argv) goto finish; } g_ptr_array_add (ssids, *argv); - } else if (!nmc->complete) - g_printerr (_("Unknown parameter: %s\n"), *argv); + } else if (!nmc->complete) { + g_string_printf (nmc->return_text, _("Error: invalid extra argument '%s'."), *argv); + nmc->return_value = NMC_RESULT_ERROR_USER_INPUT; + goto finish; + } next_arg (nmc, &argc, &argv, NULL); } diff --git a/clients/tests/test-client.check-on-disk/test_003.expected b/clients/tests/test-client.check-on-disk/test_003.expected index 20980164e9..c8cb4903b5 100644 --- a/clients/tests/test-client.check-on-disk/test_003.expected +++ b/clients/tests/test-client.check-on-disk/test_003.expected @@ -892,9 +892,9 @@ GENERAL.ZONE: -- GENERAL.MASTER-PATH: -- <<< -size: 1459 +size: 1409 location: clients/tests/test-client.py:951:test_003()/31 -cmd: $NMCLI -f ALL dev s eth0 +cmd: $NMCLI -f ALL dev status lang: C returncode: 0 stdout: 1272 bytes @@ -907,14 +907,9 @@ wlan1 wifi unavailable unknown unknown /org/freedesk wlan1 wifi unavailable unknown unknown /org/freedesktop/NetworkManager/Devices/5 -- -- -- <<< -stderr: 24 bytes ->>> -Unknown parameter: eth0 - -<<< -size: 1474 +size: 1424 location: clients/tests/test-client.py:951:test_003()/32 -cmd: $NMCLI -f ALL dev s eth0 +cmd: $NMCLI -f ALL dev status lang: pl_PL.UTF-8 returncode: 0 stdout: 1277 bytes @@ -926,11 +921,6 @@ wlan0 wifi niedostępne nieznane nieznane /org/freedes wlan1 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/4 -- -- -- wlan1 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/5 -- -- -- -<<< -stderr: 24 bytes ->>> -Nieznany parametr: eth0 - <<< size: 3494 location: clients/tests/test-client.py:954:test_003()/33 @@ -1782,9 +1772,9 @@ GENERAL.ZONE: -- GENERAL.MASTER-PATH: -- <<< -size: 1459 +size: 1409 location: clients/tests/test-client.py:951:test_003()/54 -cmd: $NMCLI -f ALL dev s eth0 +cmd: $NMCLI -f ALL dev status lang: C returncode: 0 stdout: 1272 bytes @@ -1797,14 +1787,9 @@ wlan1 wifi unavailable unknown unknown /org/freedesk wlan1 wifi unavailable unknown unknown /org/freedesktop/NetworkManager/Devices/5 -- -- -- <<< -stderr: 24 bytes ->>> -Unknown parameter: eth0 - -<<< -size: 1474 +size: 1424 location: clients/tests/test-client.py:951:test_003()/55 -cmd: $NMCLI -f ALL dev s eth0 +cmd: $NMCLI -f ALL dev status lang: pl_PL.UTF-8 returncode: 0 stdout: 1277 bytes @@ -1816,11 +1801,6 @@ wlan0 wifi niedostępne nieznane nieznane /org/freedes wlan1 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/4 -- -- -- wlan1 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/5 -- -- -- -<<< -stderr: 24 bytes ->>> -Nieznany parametr: eth0 - <<< size: 3494 location: clients/tests/test-client.py:954:test_003()/56 diff --git a/clients/tests/test-client.py b/clients/tests/test-client.py index ff5a3604dd..307a1182f3 100755 --- a/clients/tests/test-client.py +++ b/clients/tests/test-client.py @@ -947,7 +947,7 @@ class TestNmcli(NmTestBase): self.call_nmcli_l(['con', 's', 'ethernet'], replace_stdout = replace_stdout) - self.call_nmcli_l(['-f', 'ALL', 'dev', 's', 'eth0'], + self.call_nmcli_l(['-f', 'ALL', 'dev', 'status'], replace_stdout = replace_stdout) self.call_nmcli_l(['-f', 'ALL', 'dev', 'show', 'eth0'], From 72e0b522ff8a7f3e9b4c20aa29d6b179a616c5f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 10:08:41 +0200 Subject: [PATCH 08/74] shared: add NM_HASH_SEED_16() macro --- shared/nm-glib-aux/nm-hash-utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index f13e0b6d56..aa0a3d7c0a 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -26,6 +26,9 @@ /*****************************************************************************/ +#define NM_HASH_SEED_16(a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, aa, ab, ac, ad, ae, af) \ + ((const guint8[16]) { a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, aa, ab, ac, ad, ae, af }) + void nm_hash_siphash42_init (CSipHash *h, guint static_seed); /* Siphash24 of binary buffer @arr and @len, using the randomized seed from From 47fc1a4293437a88adfd247734e32fa1b86ca7a9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 30 Jul 2019 18:38:48 +0200 Subject: [PATCH 09/74] wireguard: fix crash in _auto_default_route_init() #3 0x00007fb0aa9e7d3d in g_return_if_fail_warning (log_domain=log_domain@entry=0x562295fd5ee3 "libnm", pretty_function=pretty_function@entry=0x562295fd71d0 <__func__.35180> "_connection_get_setting_check", expression=expression@entry=0x562295f8edf7 "NM_IS_CONNECTION (connection)") at ../glib/gmessages.c:2767 #4 0x0000562295df151a in _connection_get_setting_check (connection=0x0, setting_type=0x562297b17050 [NMSettingWireGuard/NMSetting]) at libnm-core/nm-connection.c:207 #5 0x0000562295df151a in _connection_get_setting_check (connection=0x0, setting_type=0x562297b17050 [NMSettingWireGuard/NMSetting]) at libnm-core/nm-connection.c:205 #6 0x0000562295ef132a in _nm_connection_get_setting (type=, connection=0x0) at ./libnm-core/nm-core-internal.h:483 #7 0x0000562295ef132a in _auto_default_route_init (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard]) at src/devices/nm-device-wireguard.c:443 #8 0x0000562295ef1b98 in coerce_route_table (device=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2, route_table=0, is_user_config=) at src/devices/nm-device-wireguard.c:565 #9 0x0000562295ea42ae in _get_route_table (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard], addr_family=addr_family@entry=2) at src/devices/nm-device.c:2311 #10 0x0000562295ea4593 in nm_device_get_route_table (self=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2) at src/devices/nm-device.c:2338 #11 0x0000562295eabde0 in ip_config_merge_and_apply (self=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2, commit=1) at src/devices/nm-device.c:7590 #12 0x0000562295ed2f0b in device_link_changed (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard]) at src/devices/nm-device.c:3939 #13 0x00007fb0aa9dc7db in g_idle_dispatch (source=source@entry=0x562297bf0b30, callback=0x562295ed2880 , user_data=0x562297bf82b0) at ../glib/gmain.c:5627 #14 0x00007fb0aa9dfedd in g_main_dispatch (context=0x562297a28090) at ../glib/gmain.c:3189 #15 0x00007fb0aa9dfedd in g_main_context_dispatch (context=context@entry=0x562297a28090) at ../glib/gmain.c:3854 #16 0x00007fb0aa9e0270 in g_main_context_iterate (context=0x562297a28090, block=block@entry=1, dispatch=dispatch@entry=1, self=) at ../glib/gmain.c:3927 #17 0x00007fb0aa9e05a3 in g_main_loop_run (loop=0x562297a0b380) at ../glib/gmain.c:4123 #18 0x0000562295d0b147 in main (argc=, argv=) at src/main.c:465 https://bugzilla.redhat.com/show_bug.cgi?id=1734383 --- src/devices/nm-device-wireguard.c | 63 +++++++++++++++++-------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index d36573cf25..e997d2e823 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -423,10 +423,10 @@ _auto_default_route_init (NMDeviceWireGuard *self) { NMDeviceWireGuardPrivate *priv = NM_DEVICE_WIREGUARD_GET_PRIVATE (self); NMConnection *connection; - NMSettingWireGuard *s_wg; - gboolean enabled_v4; - gboolean enabled_v6; + gboolean enabled_v4 = FALSE; + gboolean enabled_v6 = FALSE; gboolean refreshing_only; + guint32 new_fwmark = 0; guint32 old_fwmark; char sbuf1[100]; @@ -436,40 +436,47 @@ _auto_default_route_init (NMDeviceWireGuard *self) refreshing_only = priv->auto_default_route_initialized && priv->auto_default_route_refresh; - priv->auto_default_route_refresh = FALSE; - - connection = nm_device_get_applied_connection (NM_DEVICE (self)); - - s_wg = _nm_connection_get_setting (connection, NM_TYPE_SETTING_WIREGUARD); old_fwmark = priv->auto_default_route_fwmark; - priv->auto_default_route_fwmark = nm_setting_wireguard_get_fwmark (s_wg); + connection = nm_device_get_applied_connection (NM_DEVICE (self)); + if (connection) { + NMSettingWireGuard *s_wg; - _auto_default_route_get_enabled (s_wg, - connection, - &enabled_v4, - &enabled_v6); + s_wg = _nm_connection_get_setting (connection, NM_TYPE_SETTING_WIREGUARD); + + new_fwmark = nm_setting_wireguard_get_fwmark (s_wg); + + _auto_default_route_get_enabled (s_wg, + connection, + &enabled_v4, + &enabled_v6); + } + + if ( ( enabled_v4 + || enabled_v6) + && new_fwmark == 0u) { + if (refreshing_only) + new_fwmark = old_fwmark; + else + new_fwmark = _auto_default_route_find_unused_table (nm_device_get_platform (NM_DEVICE (self))); + } + + priv->auto_default_route_refresh = FALSE; + priv->auto_default_route_fwmark = new_fwmark; priv->auto_default_route_enabled_4 = enabled_v4; priv->auto_default_route_enabled_6 = enabled_v6; priv->auto_default_route_initialized = TRUE; - if ( ( priv->auto_default_route_enabled_4 - || priv->auto_default_route_enabled_6) - && priv->auto_default_route_fwmark == 0u) { - if (refreshing_only) - priv->auto_default_route_fwmark = old_fwmark; - else - priv->auto_default_route_fwmark = _auto_default_route_find_unused_table (nm_device_get_platform (NM_DEVICE (self))); + if (connection) { + _LOGT (LOGD_DEVICE, + "auto-default-route is %s for IPv4 and %s for IPv6%s", + priv->auto_default_route_enabled_4 ? "enabled" : "disabled", + priv->auto_default_route_enabled_6 ? "enabled" : "disabled", + priv->auto_default_route_enabled_4 || priv->auto_default_route_enabled_6 + ? nm_sprintf_buf (sbuf1, " (fwmark 0x%x)", priv->auto_default_route_fwmark) + : ""); } - - _LOGT (LOGD_DEVICE, - "auto-default-route is %s for IPv4 and %s for IPv6%s", - priv->auto_default_route_enabled_4 ? "enabled" : "disabled", - priv->auto_default_route_enabled_6 ? "enabled" : "disabled", - priv->auto_default_route_enabled_4 || priv->auto_default_route_enabled_6 - ? nm_sprintf_buf (sbuf1, " (fwmark 0x%x)", priv->auto_default_route_fwmark) - : ""); } static GPtrArray * From dc219662fa31b6bc453c177c10e03442483de21e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 30 Jul 2019 18:50:15 +0200 Subject: [PATCH 10/74] wireguard: clear cached auto-default-route setting in act_stage1_prepare() We call _auto_default_route_init() at various places, for example during coerce_route_table(). We cannot be sure that we don't call it before activation starts (before stage1) or after device-cleanup. That means, upon activation, we need to clear the state first. Do that in act_stage1_prepare(). --- src/devices/nm-device-wireguard.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index e997d2e823..0f357a1106 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1624,6 +1624,17 @@ link_config_delayed_resolver_cb (gpointer user_data) return G_SOURCE_REMOVE; } +static NMActStageReturn +act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) +{ + NMDeviceWireGuardPrivate *priv = NM_DEVICE_WIREGUARD_GET_PRIVATE (device); + + priv->auto_default_route_initialized = FALSE; + priv->auto_default_route_priority_initialized = FALSE; + + return NM_DEVICE_CLASS (nm_device_wireguard_parent_class)->act_stage1_prepare (device, out_failure_reason); +} + static NMActStageReturn act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) @@ -2064,6 +2075,7 @@ nm_device_wireguard_class_init (NMDeviceWireGuardClass *klass) device_class->connection_type_check_compatible = NM_SETTING_WIREGUARD_SETTING_NAME; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_WIREGUARD); + device_class->act_stage1_prepare = act_stage1_prepare; device_class->state_changed = device_state_changed; device_class->create_and_realize = create_and_realize; device_class->act_stage2_config = act_stage2_config; From cfb497e49936d36867ae3175537c7d4f77259655 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 10:01:04 +0200 Subject: [PATCH 11/74] wireguard: use fixed fwmark/rule-priority for auto-default-route With "wireguard.ip4-auto-default-route" and "wireguard.ip6-auto-default-route", NetworkManager automatically adds policy routing rules for the default route. For that, it needs to pick (up to) two numbers: - the fwmark. This is used both for WireGuard's fwmark setting and is also the number of the routing table where the default-route is added. - the rule priority. NetworkManager adds two policy routing rules, and we need to place them somewhere before the default rules (at 32766). Previously, we looked at exiting platform configuration and picked numbers that were not yet used. However, during restart of NetworkManager, we leave the interface up and after restart we will take over the previous configuration. At that point, we need to choose the same fwmark/priority, otherwise the configuration changes. But since we picked numbers that were not yet used, we would always choose different numbers. For routing rules that meant that after restart a second pair of rules was added. We possibly could store this data in the device's state-file. But that is complex. Instead, just pick numbers deterministically based on the connection's UUID. If the picked numbers are not suitable, then the user can still work around that: - for fwmark, the user can explicitly configure wireguard.fwmark setting. - for the policy routes, the user can explicitly add the rules with the desired priorirites (arguably, currently the default-route cannot be added like a regular route, so the table cannot be set. Possibly the user would have to add two /1 routes instead with suppress_prefixlength=1). --- src/devices/nm-device-wireguard.c | 171 ++++++------------------------ 1 file changed, 32 insertions(+), 139 deletions(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index 0f357a1106..f4da539356 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -265,157 +265,51 @@ done: *out_enabled_v6 = (enabled_v6 == TRUE); } -static guint32 -_auto_default_route_find_unused_table (NMPlatform *platform) -{ - guint32 table; - int is_ipv4; - - for (table = 51820; TRUE; table++) { - const NMDedupMultiHeadEntry *head_entry; - const guint32 table_coerced = nm_platform_route_table_coerce (table); - NMDedupMultiIter iter; - const NMPObject *plobj; - - /* find a table/fwmark that is not yet in use. */ - - for (is_ipv4 = 0; is_ipv4 < 2; is_ipv4++) { - head_entry = nm_platform_lookup_object (platform, - is_ipv4 - ? NMP_OBJECT_TYPE_IP4_ROUTE - : NMP_OBJECT_TYPE_IP6_ROUTE, - -1); - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - if (NMP_OBJECT_CAST_IP_ROUTE (plobj)->table_coerced == table_coerced) - goto try_next_table; - } - } - - head_entry = nm_platform_lookup_object_by_addr_family (platform, - NMP_OBJECT_TYPE_ROUTING_RULE, - AF_UNSPEC); - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - const NMPlatformRoutingRule *rr = NMP_OBJECT_CAST_ROUTING_RULE (plobj); - - if (rr->fwmark == table) - goto try_next_table; - } - - head_entry = nm_platform_lookup_obj_type (platform, NMP_OBJECT_TYPE_LINK); - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - const NMPObject *lnk_wg; - - if (plobj->link.type != NM_LINK_TYPE_WIREGUARD) - continue; - - lnk_wg = plobj->_link.netlink.lnk; - - if (!lnk_wg) - continue; - - if (NMP_OBJECT_GET_TYPE (lnk_wg) != NMP_OBJECT_TYPE_LNK_WIREGUARD) - continue; - - if (NMP_OBJECT_CAST_LNK_WIREGUARD (lnk_wg)->fwmark == table) - goto try_next_table; - } - - return table; -try_next_table: - ; - } -} - -#define PRIO_WIDTH ((guint32) 2) - -static gboolean -_auto_default_route_find_priority_exists (const NMDedupMultiHeadEntry *head_entry, - guint32 priority) -{ - NMDedupMultiIter iter; - const NMPObject *plobj; - - nmp_cache_iter_for_each (&iter, head_entry, &plobj) { - const NMPlatformRoutingRule *rr = NMP_OBJECT_CAST_ROUTING_RULE (plobj); - - /* we don't differenciate between IPv4 vs. IPv6. There should be no - * conflicting rules with the same priority. */ - if ( rr->priority >= priority - && rr->priority < priority + PRIO_WIDTH) - return TRUE; - } - - return FALSE; -} +#define AUTO_RANDOM_RANGE 500u static guint32 -_auto_default_route_find_priority (NMPlatform *platform, - const char *uuid) +_auto_default_route_get_auto_fwmark (const char *uuid) { - const NMDedupMultiHeadEntry *head_entry; guint64 rnd_seed; - const guint32 PRIME_NUMBER = 1111567573u; - const guint32 RANGE_TOP = ((32766u - 2u * PRIO_WIDTH) / PRIO_WIDTH); - const guint32 RANGE_LEN1 = 200u; - const guint32 RANGE_LEN2 = (RANGE_TOP - 100u) - RANGE_LEN1; - guint32 range_len; - guint32 range_top; - guint32 prio_candidate = 0; - guint32 i_step; - guint32 i; - /* For the auto-default-route policy routing rule we add 4 rules (2 Ipv4 and 2 IPv6). - * Hence, we choose a priority for the first (of the two rules) and the second - * rule gets priority + 1. - * We want a priority that is - * - unused so far. - * - smaller than 32766u (which is the priority of the default rules for IPv4 and IPv6) - * - stable for each connection but different between connections (we hash the UUID - * as a "random" seed) - * - if possible, close to 32766u (RANGE_LEN1). Only otherwise fallback to the entire - * range (RANGE_LEN2). + /* we use the generated number as fwmark but also as routing table for + * the default-route. + * + * We pick a number + * + * - based on the connection's UUID (as stable seed). + * - larger than 51820u (arbitrarily) + * - one out of AUTO_RANDOM_RANGE */ - rnd_seed = c_siphash_hash ((const guint8 [16]) { 0xb9, 0x39, 0x8e, 0xed, 0x15, 0xb3, 0xd1, 0xc4, 0x5f, 0x45, 0x00, 0x4f, 0xec, 0xc2, 0x2b, 0x7e }, + rnd_seed = c_siphash_hash (NM_HASH_SEED_16 (0xb9, 0x39, 0x8e, 0xed, 0x15, 0xb3, 0xd1, 0xc4, 0x5f, 0x45, 0x00, 0x4f, 0xec, 0xc2, 0x2b, 0x7e), (const guint8 *) uuid, uuid ? strlen (uuid) + 1u : 0u); - head_entry = nm_platform_lookup_object_by_addr_family (platform, - NMP_OBJECT_TYPE_ROUTING_RULE, - AF_UNSPEC); + return 51820u + (rnd_seed % AUTO_RANDOM_RANGE); +} - range_len = RANGE_LEN1; - range_top = RANGE_TOP; +#define PRIO_WIDTH 2u -again: - i_step = ((guint32) rnd_seed) % range_len; - for (i = 0; i < range_len; i++) { +static guint32 +_auto_default_route_get_auto_priority (const char *uuid) +{ + const guint32 RANGE_TOP = 32766u - 1000u; + guint64 rnd_seed; - /* we sample the range in a stable, but somewhat arbitrary order to - * find an unused priority. */ - i_step = (i_step + PRIME_NUMBER) % range_len; + /* we pick a priority for the routing rules as follows: + * + * - use the connection's UUID as stable seed for the "random" number. + * - have it smaller than RANGE_TOP (32766u - 1000u), where 32766u is the priority of the default + * rules + * - we add 2 rules (PRIO_WIDTH). Hence only pick even priorites. + * - pick one out of AUTO_RANDOM_RANGE. */ - nm_assert (i_step < range_top); + rnd_seed = c_siphash_hash (NM_HASH_SEED_16 (0x99, 0x22, 0x4d, 0x7c, 0x37, 0xda, 0x8e, 0x7b, 0x2f, 0x55, 0x16, 0x7b, 0x75, 0xda, 0x42, 0xdc), + (const guint8 *) uuid, + uuid ? strlen (uuid) + 1u : 0u); - prio_candidate = (range_top - i_step) * PRIO_WIDTH; - - nm_assert (prio_candidate < 32766u); - - if (!_auto_default_route_find_priority_exists (head_entry, prio_candidate)) - return prio_candidate; - } - - if (range_len == RANGE_LEN1) { - /* within the narrow range close to RANGE_TOP we couldn't find any unused - * priority. Retry with the entire range... */ - range_len = RANGE_LEN2; - range_top -= RANGE_LEN1; - goto again; - } - - /* Couldn't find an unused one? Very odd, this really should not happen unless there - * are thousands of rules already. Just pick the last one we sampled. */ - return prio_candidate; + return RANGE_TOP - (((rnd_seed % (PRIO_WIDTH * AUTO_RANDOM_RANGE)) / PRIO_WIDTH) * PRIO_WIDTH); } static void @@ -459,7 +353,7 @@ _auto_default_route_init (NMDeviceWireGuard *self) if (refreshing_only) new_fwmark = old_fwmark; else - new_fwmark = _auto_default_route_find_unused_table (nm_device_get_platform (NM_DEVICE (self))); + new_fwmark = _auto_default_route_get_auto_fwmark (nm_connection_get_uuid (connection)); } priv->auto_default_route_refresh = FALSE; @@ -513,8 +407,7 @@ get_extra_rules (NMDevice *device) if (priv->auto_default_route_priority_initialized) priority = priv->auto_default_route_priority; else { - priority = _auto_default_route_find_priority (nm_device_get_platform (device), - nm_connection_get_uuid (connection)); + priority = _auto_default_route_get_auto_priority (nm_connection_get_uuid (connection)); priv->auto_default_route_priority = priority; priv->auto_default_route_priority_initialized = TRUE; } From e966f4b272331fcf99914ee2c1a980074a01ed94 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 10:17:59 +0200 Subject: [PATCH 12/74] example: print WireGuard parameters in nm-wg-set example script --- examples/python/gi/nm-wg-set | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/examples/python/gi/nm-wg-set b/examples/python/gi/nm-wg-set index 81ee5667e4..5f284c2c25 100755 --- a/examples/python/gi/nm-wg-set +++ b/examples/python/gi/nm-wg-set @@ -234,6 +234,15 @@ def secret_to_string(secret): return '' return secret +def val_to_str(val): + if val == NM.Ternary.DEFAULT: + return 'default' + if val == NM.Ternary.TRUE: + return 'true' + if val == NM.Ternary.FALSE: + return 'false' + return repr(val) + ############################################################################### def wg_read_private_key(privkey_file): @@ -273,6 +282,9 @@ def do_get(nm_client, connection): print('private-key-flags: %s' % (secret_flags_to_string(s_wg.get_private_key_flags()))) print('listen-port: %s' % (s_wg.get_listen_port())) print('fwmark: 0x%x' % (s_wg.get_fwmark())) + print('peer-routes: %s' % (val_to_str(s_wg.get_peer_routes()))) + print('ip4-auto-default-route: %s' % (val_to_str(s_wg.get_ip4_auto_default_route()))) + print('ip6-auto-default-route: %s' % (val_to_str(s_wg.get_ip6_auto_default_route()))) for i in range(s_wg.get_peers_len()): peer = s_wg.get_peer(i) print('peer[%d].public-key: %s' % (i, peer.get_public_key())) From f453eeb588390621b34d93d922ff2592aa0962a5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 31 Jul 2019 12:13:27 +0200 Subject: [PATCH 13/74] introspection/doc: better document flags argument of Update2() D-Bus command --- ...rg.freedesktop.NetworkManager.Settings.Connection.xml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml index 2cbb6c2ebc..6211274fd9 100644 --- a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml +++ b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml @@ -111,7 +111,8 @@ "0x4" (in-memory-detached), "0x8" (in-memory-only), "0x10" (volatile), - "0x20" (block-autoconnect). + "0x20" (block-autoconnect), + "0x40" (no-reapply). Unknown flags cause the call to fail. @args: optional arguments dictionary, for extensibility. Currently no arguments are accepted. Specifying unknown keys causes the call @@ -124,6 +125,12 @@ the change is only made in memory (without touching an eventual profile on disk). If neither 0x1 nor 0x2 is set, the change is made in memory only, if the connection is already in memory only. + The flags 0x4 (in-memory-detached) and 0x8 (in-memory-only) are like + "in-memory", but behave slightly different when migrating the profile + from disk to in-memory. + The flag 0x20 (block-autoconnect) blocks auto-connect on the updated + profile, and 0x40 (no-reapply) prevents "connection.zone" and "connection.metered" + properties to take effect on currently active devices. Secrets may be part of the update request, and will be either stored in persistent storage or sent to a Secret Agent for storage, depending on the flags associated with each secret. From acf3e0092ad8d52ef54644d63c8e4f7f974847c7 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 31 Jul 2019 16:12:22 +0200 Subject: [PATCH 14/74] manager: don't treat the initramfs-configured DHCP connections as generated These are special -- initramfs configured them and killed dhclient. Bad things would happen if we let the addresses expire though. Let's act as if initramfs actually passed the configuration to us. It actually tries to do so by the means of writing an ifcfg file, but that one is too broken to be useful, so the ifcfg-rh plugin ignores it. Notably, it doesn't have the actual addresses or correct BOOTPROTO. The generated connection is better. Co-authored-by: Thomas Haller --- src/nm-manager.c | 46 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 6ea535021a..64cdb9aed2 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2698,6 +2698,7 @@ recheck_assume_connection (NMManager *self, gboolean was_unmanaged = FALSE; gboolean generated = FALSE; NMDeviceState state; + gboolean activation_type_assume; g_return_val_if_fail (NM_IS_MANAGER (self), FALSE); g_return_val_if_fail (NM_IS_DEVICE (device), FALSE); @@ -2721,10 +2722,42 @@ recheck_assume_connection (NMManager *self, if (!sett_conn) return FALSE; + activation_type_assume = !generated; + + if (state == NM_DEVICE_STATE_UNMANAGED) { + gs_free char *initramfs_lease = g_strdup_printf (RUNSTATEDIR "/initramfs/net.%s.lease", + nm_device_get_iface (device)); + gs_free char *connection_lease = g_strdup_printf (NMRUNDIR "/dhclient-%s-%s.lease", + nm_settings_connection_get_uuid (sett_conn), + nm_device_get_iface (device)); + + if (rename (initramfs_lease, connection_lease) == 0) { + /* + * We've managed to steal the lease used by initramfs before it + * killed off the dhclient. We need to take ownership of the configured + * connection and act like the device was configured by us. + * Otherwise the address would just expire. + */ + _LOG2I (LOGD_DEVICE, device, "assume: taking over an initramfs-configured connection"); + activation_type_assume = TRUE; + + if (generated) { + nm_settings_connection_update (sett_conn, + NULL, + NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP, + 0, + NM_SETTINGS_CONNECTION_INT_FLAGS_VOLATILE, + NM_SETTINGS_CONNECTION_UPDATE_REASON_NONE, + "assume-initrd", + NULL); + } + } + } + nm_device_sys_iface_state_set (device, - generated - ? NM_DEVICE_SYS_IFACE_STATE_EXTERNAL - : NM_DEVICE_SYS_IFACE_STATE_ASSUME); + activation_type_assume + ? NM_DEVICE_SYS_IFACE_STATE_ASSUME + : NM_DEVICE_SYS_IFACE_STATE_EXTERNAL); /* Move device to DISCONNECTED to activate the connection */ if (state == NM_DEVICE_STATE_UNMANAGED) { @@ -2768,8 +2801,8 @@ recheck_assume_connection (NMManager *self, NULL, device, subject, - generated ? NM_ACTIVATION_TYPE_EXTERNAL : NM_ACTIVATION_TYPE_ASSUME, - generated ? NM_ACTIVATION_REASON_EXTERNAL : NM_ACTIVATION_REASON_ASSUME, + activation_type_assume ? NM_ACTIVATION_TYPE_ASSUME : NM_ACTIVATION_TYPE_EXTERNAL, + activation_type_assume ? NM_ACTIVATION_REASON_ASSUME : NM_ACTIVATION_REASON_EXTERNAL, NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY, &error); @@ -2785,7 +2818,8 @@ recheck_assume_connection (NMManager *self, NM_DEVICE_STATE_REASON_CONFIG_FAILED); } - if (generated) { + if ( generated + && !activation_type_assume) { _LOG2D (LOGD_DEVICE, device, "assume: deleting generated connection after assuming failed"); nm_settings_connection_delete (sett_conn, FALSE); } else { From 3cb4b36261684aa3d2676f922c6d53bc31085153 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 30 Jul 2019 11:03:59 +0200 Subject: [PATCH 15/74] device: check platform link compatibility when setting nm-owned flag We set nm-owned to indicate whether a software device was created by NM or it was pre-existing. When checking the existence, we must verify also whether the link type is compatible with the device, otherwise it is possible to match unrelated interfaces. For example, when checking for the existence of an ovs-bridge (which is not compatible with any platform link) we could match a unrelated platform link with the same name. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 --- src/devices/nm-device.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index feb5110d30..f6a3c3f8fa 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4153,13 +4153,14 @@ nm_device_create_and_realize (NMDevice *self, { nm_auto_nmpobj const NMPObject *plink_keep_alive = NULL; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const NMPlatformLink *plink = NULL; + const NMPlatformLink *plink; /* Must be set before device is realized */ - priv->nm_owned = !nm_platform_link_get_by_ifname (nm_device_get_platform (self), priv->iface); - + plink = nm_platform_link_get_by_ifname (nm_device_get_platform (self), priv->iface); + priv->nm_owned = !plink || !link_type_compatible (self, plink->type, NULL, NULL); _LOGD (LOGD_DEVICE, "create (is %snm-owned)", priv->nm_owned ? "" : "not "); + plink = NULL; /* Create any resources the device needs */ if (NM_DEVICE_GET_CLASS (self)->create_and_realize) { if (!NM_DEVICE_GET_CLASS (self)->create_and_realize (self, connection, parent, &plink, error)) From 57e3734b6cc1bb453216c7e2150a698114507a46 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 31 Jul 2019 11:40:35 +0200 Subject: [PATCH 16/74] device: fix releasing slaves Not all masters type have a platform link and so it's wrong to check for it to decide whether the slave should be really released. Move the check to master devices that need it (bond, bridge and team). OVS ports don't need the check because they don't call to platform to remove a slave. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 --- src/devices/nm-device-bond.c | 6 ++++++ src/devices/nm-device-bridge.c | 6 ++++++ src/devices/nm-device.c | 7 +------ src/devices/team/nm-device-team.c | 6 ++++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index fd79348d2c..50494526a9 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -414,6 +414,12 @@ release_slave (NMDevice *device, gboolean success; gs_free char *address = NULL; int ifindex_slave; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; ifindex_slave = nm_device_get_ip_ifindex (slave); diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index ade9eb0d8f..91d824b35b 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -626,6 +626,12 @@ release_slave (NMDevice *device, NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); gboolean success; int ifindex_slave; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; ifindex_slave = nm_device_get_ip_ifindex (slave); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f6a3c3f8fa..cf80774298 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4987,7 +4987,6 @@ nm_device_master_release_slaves (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMDeviceStateReason reason; - gboolean configure = TRUE; CList *iter, *safe; /* Don't release the slaves if this connection doesn't belong to NM. */ @@ -4998,14 +4997,10 @@ nm_device_master_release_slaves (NMDevice *self) if (priv->state == NM_DEVICE_STATE_FAILED) reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; - if ( priv->ifindex <= 0 - || !nm_platform_link_get (nm_device_get_platform (self), priv->ifindex)) - configure = FALSE; - c_list_for_each_safe (iter, safe, &priv->slaves) { SlaveInfo *info = c_list_entry (iter, SlaveInfo, lst_slave); - nm_device_master_release_one_slave (self, info->slave, configure, reason); + nm_device_master_release_one_slave (self, info->slave, TRUE, reason); } } diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index a60a9fda5d..4661a84034 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -775,6 +775,12 @@ release_slave (NMDevice *device, NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); gboolean success; int ifindex_slave; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; ifindex_slave = nm_device_get_ip_ifindex (slave); From 72e604c8e4a0d89c9474a7048d330a9bc8406812 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 14:56:07 +0200 Subject: [PATCH 17/74] device: avoid unnecessary check for existing device in release_slave() implementations --- src/devices/nm-device-bond.c | 10 ++++++---- src/devices/nm-device-bridge.c | 10 ++++++---- src/devices/team/nm-device-team.c | 10 ++++++---- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 50494526a9..7626b04a00 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -416,10 +416,12 @@ release_slave (NMDevice *device, int ifindex_slave; int ifindex; - ifindex = nm_device_get_ifindex (device); - if ( ifindex <= 0 - || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) - configure = FALSE; + if (configure) { + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; + } ifindex_slave = nm_device_get_ip_ifindex (slave); diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 91d824b35b..8f3a4045ba 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -628,10 +628,12 @@ release_slave (NMDevice *device, int ifindex_slave; int ifindex; - ifindex = nm_device_get_ifindex (device); - if ( ifindex <= 0 - || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) - configure = FALSE; + if (configure) { + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; + } ifindex_slave = nm_device_get_ip_ifindex (slave); diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 4661a84034..79ac8935bb 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -777,10 +777,12 @@ release_slave (NMDevice *device, int ifindex_slave; int ifindex; - ifindex = nm_device_get_ifindex (device); - if ( ifindex <= 0 - || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) - configure = FALSE; + if (configure) { + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; + } ifindex_slave = nm_device_get_ip_ifindex (slave); From ece270ea5fa058e3c8448c3dfe5676ff67311b6c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 15:04:39 +0200 Subject: [PATCH 18/74] core/lldp: fix memleak in _lldp_attr_take_str_ptr() Valgrind complains: ==26355== 32 bytes in 2 blocks are definitely lost in loss record 2,829 of 6,716 ==26355== at 0x4838748: malloc (vg_replace_malloc.c:308) ==26355== by 0x483AD63: realloc (vg_replace_malloc.c:836) ==26355== by 0x4F6AD4F: g_realloc (in /usr/lib64/libglib-2.0.so.0.6000.6) ==26355== by 0x4F87B33: ??? (in /usr/lib64/libglib-2.0.so.0.6000.6) ==26355== by 0x4F87B96: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.6000.6) ==26355== by 0x2D66E1: nm_utils_buf_utf8safe_escape (nm-shared-utils.c:1911) ==26355== by 0x4113B0: lldp_neighbor_new (nm-lldp-listener.c:676) ==26355== by 0x412788: process_lldp_neighbor (nm-lldp-listener.c:882) ==26355== by 0x4135CF: lldp_event_handler (nm-lldp-listener.c:931) ==26355== by 0x422CDB: lldp_callback (sd-lldp.c:50) ==26355== by 0x4235F9: lldp_add_neighbor (sd-lldp.c:166) ==26355== by 0x423679: lldp_handle_datagram (sd-lldp.c:189) ==26355== by 0x423C8B: lldp_receive_datagram (sd-lldp.c:235) ==26355== by 0x2F887A: source_dispatch (sd-event.c:2832) ==26355== by 0x2FAD43: sd_event_dispatch (sd-event.c:3245) ==26355== by 0x2D9237: event_dispatch (nm-sd.c:51) ==26355== by 0x4F64EDC: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.6000.6) ==26355== by 0x4F6526F: ??? (in /usr/lib64/libglib-2.0.so.0.6000.6) ==26355== by 0x4F655A2: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.6000.6) ==26355== by 0x140932: main (main.c:465) ==26355== --- src/devices/nm-lldp-listener.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-lldp-listener.c b/src/devices/nm-lldp-listener.c index c0d783158c..2dc38d4ca8 100644 --- a/src/devices/nm-lldp-listener.c +++ b/src/devices/nm-lldp-listener.c @@ -232,8 +232,10 @@ _lldp_attr_take_str_ptr (LldpAttrData *pdata, LldpAttrId attr_id, char *str) pdata = &pdata[attr_id]; /* we ignore duplicate fields silently. */ - if (pdata->attr_type != LLDP_ATTR_TYPE_NONE) + if (pdata->attr_type != LLDP_ATTR_TYPE_NONE) { + g_free (str); return; + } pdata->attr_type = LLDP_ATTR_TYPE_STRING; pdata->v_string = str; From 0fbb54839e3f1b5309fa3d81cf8821b2fa1ed1e0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 15:05:27 +0200 Subject: [PATCH 19/74] core/lldp: minor cleanup in _lldp_attr_*() - use nm_g_variant_unref_floating() - rename _lldp_attr_take_str_ptr() to _lldp_attr_set_str_take(). The new name has the same "_lldp_attr_set_" prefix as other setters. Also, with the previous name it is unclear why it takes a "str-ptr". - setting the same attribute multiple times, ignores all but the first value. Avoid cloning the string in that case, and explicitly choose the set or take function. --- src/devices/nm-lldp-listener.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-lldp-listener.c b/src/devices/nm-lldp-listener.c index 2dc38d4ca8..469ccce165 100644 --- a/src/devices/nm-lldp-listener.c +++ b/src/devices/nm-lldp-listener.c @@ -224,7 +224,7 @@ _lldp_attr_set_str (LldpAttrData *pdata, LldpAttrId attr_id, const char *v_strin } static void -_lldp_attr_take_str_ptr (LldpAttrData *pdata, LldpAttrId attr_id, char *str) +_lldp_attr_set_str_take (LldpAttrData *pdata, LldpAttrId attr_id, char *str) { nm_assert (pdata); nm_assert (_lldp_attr_id_to_type (attr_id) == LLDP_ATTR_TYPE_STRING); @@ -267,8 +267,7 @@ _lldp_attr_set_vardict (LldpAttrData *pdata, LldpAttrId attr_id, GVariant *varia /* we ignore duplicate fields silently */ if (pdata->attr_type != LLDP_ATTR_TYPE_NONE) { - if (g_variant_is_floating (variant)) - g_variant_unref (variant); + nm_g_variant_unref_floating (variant); return; } @@ -687,9 +686,10 @@ lldp_neighbor_new (sd_lldp_neighbor *neighbor_sd, GError **error) g_variant_dict_end (&dict)); _lldp_attr_set_uint32 (neigh->attrs, LLDP_ATTR_ID_IEEE_802_1_VID, vid); - _lldp_attr_take_str_ptr (neigh->attrs, - LLDP_ATTR_ID_IEEE_802_1_VLAN_NAME, - name_to_free ?: g_strdup (name)); + if (name_to_free) + _lldp_attr_set_str_take (neigh->attrs, LLDP_ATTR_ID_IEEE_802_1_VLAN_NAME, name_to_free); + else + _lldp_attr_set_str (neigh->attrs, LLDP_ATTR_ID_IEEE_802_1_VLAN_NAME, name); break; } default: From 9a229241f96ce48f77baa442b1b6aeac9d8489cb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:24:55 +0200 Subject: [PATCH 20/74] shared: fix non-serious bug with bogus condition in assertion in nm_key_file_db_ref() --- shared/nm-glib-aux/nm-keyfile-aux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-keyfile-aux.c b/shared/nm-glib-aux/nm-keyfile-aux.c index 0257bcca7f..989c773f23 100644 --- a/shared/nm-glib-aux/nm-keyfile-aux.c +++ b/shared/nm-glib-aux/nm-keyfile-aux.c @@ -125,7 +125,7 @@ nm_key_file_db_ref (NMKeyFileDB *self) g_return_val_if_fail (_IS_KEY_FILE_DB (self, FALSE, TRUE), NULL); - nm_assert (self->ref_count <= G_MAXUINT); + nm_assert (self->ref_count < G_MAXUINT); self->ref_count++; return self; } From 2ea3c237235312f75498bd55deca128036029b31 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:28:58 +0200 Subject: [PATCH 21/74] core/trivial: fix whitespace --- src/platform/wifi/nm-wifi-utils-nl80211.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c index 164dada4e9..c4a8bd8a7d 100644 --- a/src/platform/wifi/nm-wifi-utils-nl80211.c +++ b/src/platform/wifi/nm-wifi-utils-nl80211.c @@ -284,8 +284,8 @@ nl80211_get_wake_on_wlan_handler (struct nl_msg *msg, void *arg) struct genlmsghdr *gnlh = nlmsg_data (nlmsg_hdr (msg)); nla_parse_arr (attrs, - genlmsg_attrdata(gnlh, 0), - genlmsg_attrlen(gnlh, 0), + genlmsg_attrdata (gnlh, 0), + genlmsg_attrlen (gnlh, 0), NULL); if (!attrs[NL80211_ATTR_WOWLAN_TRIGGERS]) @@ -363,7 +363,7 @@ wifi_nl80211_set_wake_on_wlan (NMWifiUtils *data, NMSettingWirelessWakeOnWLan wo if (NM_FLAGS_HAS (wowl, NM_SETTING_WIRELESS_WAKE_ON_WLAN_RFKILL_RELEASE)) NLA_PUT_FLAG (msg, NL80211_WOWLAN_TRIG_RFKILL_RELEASE); - nla_nest_end(msg, triggers); + nla_nest_end (msg, triggers); err = nl80211_send_and_recv (self, msg, NULL, NULL); From 210d7eb5282ca7c21d5d1fd5b10ee37d297fddf2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:38:10 +0200 Subject: [PATCH 22/74] ifcfg-rh: drop unreachable code in make_wpa_setting() This triggers a coverity warning because we above already check that not all relevant keys are NULL together. Work around warning by modifying the code. --- src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 900a3fc1de..2e0221420b 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -3629,7 +3629,10 @@ make_wpa_setting (shvarFile *ifcfg, wpa_sae = nm_streq0 (v, "SAE"); wpa_eap = nm_streq0 (v, "WPA-EAP"); ieee8021x = nm_streq0 (v, "IEEE8021X"); - if (!wpa_psk && !wpa_sae && !wpa_eap && !ieee8021x) + if ( !wpa_psk + && !wpa_sae + && !wpa_eap + && !ieee8021x) return NULL; /* Not WPA or Dynamic WEP */ /* WPS */ @@ -3693,7 +3696,9 @@ make_wpa_setting (shvarFile *ifcfg, g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "sae", NULL); else g_assert_not_reached (); - } else if (wpa_eap || ieee8021x) { + } else { + nm_assert (wpa_eap || ieee8021x); + /* Adhoc mode is mutually exclusive with any 802.1x-based authentication */ if (adhoc) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, @@ -3710,10 +3715,6 @@ make_wpa_setting (shvarFile *ifcfg, g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, lower, NULL); } - } else { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "Unknown wireless KEY_MGMT type '%s'", v); - return NULL; } i_val = NM_SETTING_WIRELESS_SECURITY_PMF_DEFAULT; From 43575513ca91993c5a5995d492361c477b05a99d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:40:39 +0200 Subject: [PATCH 23/74] ifcfg-rh: drop g_assert_not_reached() that clearly cannot be reached Use nm_assert() which is disabled in production builds. --- src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 2e0221420b..9c3ae10af8 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -3692,10 +3692,10 @@ make_wpa_setting (shvarFile *ifcfg, g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-none", NULL); else if (wpa_psk) g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-psk", NULL); - else if (wpa_sae) + else { + nm_assert (wpa_sae); g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "sae", NULL); - else - g_assert_not_reached (); + } } else { nm_assert (wpa_eap || ieee8021x); From 026739eb9faf45ade38cf450430c26d638445453 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:45:43 +0200 Subject: [PATCH 24/74] core: fix coverity warning about memset() non-char value in assertion CID 202432 (#1 of 1): Memset fill truncated (NO_EFFECT) bad_memset: Argument -559030611 in memset loses precision in memset(priv->connections_cached_list, -559030611, 8UL * (priv->connections_len + 1U)). --- src/settings/nm-settings.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 430d277693..9a17d4774c 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -2866,7 +2866,7 @@ _clear_connections_cached_list (NMSettingsPrivate *priv) * it. That is a bug, this code just tries to make it blow up * more eagerly. */ memset (priv->connections_cached_list, - 0xdeaddead, + 0x43, sizeof (NMSettingsConnection *) * (priv->connections_len + 1)); #endif From 243458836adcdb4652bcb5871b53e6f65b510985 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:48:48 +0200 Subject: [PATCH 25/74] platform: avoid coverity warning about not checking nla_nest_start() result Usually we check the result of nla_nest_start(). Also, in most cases where this function would return %NULL, it's an actual bug. That is, because our netlink message is allocated with a large buffer, and in most cases we append there a well known, small amount of data. To make coverity happy, handle the case and assert. --- src/platform/wifi/nm-wifi-utils-nl80211.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c index c4a8bd8a7d..2f30b05a2e 100644 --- a/src/platform/wifi/nm-wifi-utils-nl80211.c +++ b/src/platform/wifi/nm-wifi-utils-nl80211.c @@ -347,6 +347,8 @@ wifi_nl80211_set_wake_on_wlan (NMWifiUtils *data, NMSettingWirelessWakeOnWLan wo return FALSE; triggers = nla_nest_start (msg, NL80211_ATTR_WOWLAN_TRIGGERS); + if (!triggers) + goto nla_put_failure; if (NM_FLAGS_HAS (wowl, NM_SETTING_WIRELESS_WAKE_ON_WLAN_ANY)) NLA_PUT_FLAG (msg, NL80211_WOWLAN_TRIG_ANY); From 990a7bee9d35c19420fe87c44293e5c125cde2e5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 17:48:48 +0200 Subject: [PATCH 26/74] platform: drop checks for failure of nl80211_alloc_msg() nl80211_alloc_msg() just allocates some memory, using glib's allocators. Hence it cannot fail, and we don't need to check for that. Drop the unnecessary %NULL checks. --- src/platform/wifi/nm-wifi-utils-nl80211.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c index 2f30b05a2e..84a93f4f6b 100644 --- a/src/platform/wifi/nm-wifi-utils-nl80211.c +++ b/src/platform/wifi/nm-wifi-utils-nl80211.c @@ -343,8 +343,6 @@ wifi_nl80211_set_wake_on_wlan (NMWifiUtils *data, NMSettingWirelessWakeOnWLan wo return TRUE; msg = nl80211_alloc_msg (self, NL80211_CMD_SET_WOWLAN, 0); - if (!msg) - return FALSE; triggers = nla_nest_start (msg, NL80211_ATTR_WOWLAN_TRIGGERS); if (!triggers) @@ -636,14 +634,12 @@ nl80211_get_ap_info (NMWifiUtilsNl80211 *self, return; msg = nl80211_alloc_msg (self, NL80211_CMD_GET_STATION, 0); - if (msg) { - NLA_PUT (msg, NL80211_ATTR_MAC, ETH_ALEN, bss_info.bssid); + NLA_PUT (msg, NL80211_ATTR_MAC, ETH_ALEN, bss_info.bssid); - nl80211_send_and_recv (self, msg, nl80211_station_handler, sta_info); - if (!sta_info->signal_valid) { - /* Fall back to bss_info signal quality (both are in percent) */ - sta_info->signal = bss_info.beacon_signal; - } + nl80211_send_and_recv (self, msg, nl80211_station_handler, sta_info); + if (!sta_info->signal_valid) { + /* Fall back to bss_info signal quality (both are in percent) */ + sta_info->signal = bss_info.beacon_signal; } return; From bee0b20e3f04f93fe2ae906141c8973110b94667 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 08:29:18 +0200 Subject: [PATCH 27/74] cli: use gs_free_error in nmcli's "connections.c" --- clients/cli/connections.c | 60 +++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index 806e3504c1..c88fbd2abb 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -1298,17 +1298,16 @@ got_secrets (GObject *source_object, GAsyncResult *res, gpointer user_data) { NMRemoteConnection *remote = NM_REMOTE_CONNECTION (source_object); GetSecretsData *data = user_data; - GVariant *secrets; - GError *error = NULL; + gs_unref_variant GVariant *secrets = NULL; secrets = nm_remote_connection_get_secrets_finish (remote, res, NULL); if (secrets) { + gs_free_error GError *error = NULL; + if (!nm_connection_update_secrets (data->local, NULL, secrets, &error) && error) { g_printerr (_("Error updating secrets for %s: %s\n"), data->setting_name, error->message); - g_clear_error (&error); } - g_variant_unref (secrets); } g_main_loop_quit (data->loop); @@ -2764,6 +2763,7 @@ parse_passwords (const char *passwd_file, GError **error) g_hash_table_insert (pwds_hash, pwd_spec, g_strdup (pwd)); } + return g_steal_pointer (&pwds_hash); } @@ -2783,22 +2783,24 @@ nmc_activate_connection (NmCli *nmc, NMDevice *device = NULL; const char *spec_object = NULL; gboolean device_found; - GError *local = NULL; g_return_val_if_fail (nmc, FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - if (connection && (ifname || ap || nsp)) { + if ( connection + && ( ifname + || ap + || nsp)) { + gs_free_error GError *local = NULL; + device_found = find_device_for_connection (nmc, connection, ifname, ap, nsp, &device, &spec_object, &local); /* Virtual connection may not have their interfaces created yet */ if (!device_found && !nm_connection_is_virtual (connection)) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_CON_ACTIVATION, "%s", local->message); - g_clear_error (&local); return FALSE; } - g_clear_error (&local); } else if (ifname) { device = nm_client_get_device_by_iface (nmc->client, ifname); if (!device) { @@ -2813,11 +2815,10 @@ nmc_activate_connection (NmCli *nmc, } /* Parse passwords given in passwords file */ - pwds_hash = parse_passwords (pwds, &local); - if (local) { - g_propagate_error (error, local); + pwds_hash = parse_passwords (pwds, error); + if (!pwds_hash) return FALSE; - } + if (nmc->pwds_hash) g_hash_table_destroy (nmc->pwds_hash); nmc->pwds_hash = pwds_hash; @@ -7573,18 +7574,19 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t } } else { gs_free char *prop_name = NULL; - GError *tmp_err = NULL; + gs_free GError *tmp_err = NULL; prop_name = is_property_valid (ss, cmd_arg_p, &tmp_err); if (prop_name) { if (!nmc_setting_set_property (nmc->client, ss, prop_name, '\0', NULL, &tmp_err)) { - g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, + g_print (_("Error: failed to remove value of '%s': %s\n"), + prop_name, tmp_err->message); - g_clear_error (&tmp_err); } } else { - /* If the string is not a property, try it as a setting */ NMSetting *s_tmp; + + /* If the string is not a property, try it as a setting */ s_tmp = is_setting_valid (connection, valid_settings_main, valid_settings_slave, @@ -7592,16 +7594,17 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if (s_tmp) { /* Remove setting from the connection */ connection_remove_setting (connection, s_tmp); + /* coverity[copy_paste_error] - suppress Coverity COPY_PASTE_ERROR defect */ if (ss == menu_ctx.curr_setting) { /* If we removed the setting we are in, go up */ menu_switch_to_level0 (&nmc->nmc_config, &menu_ctx, BASE_PROMPT); nmc_tab_completion.setting = NULL; /* for TAB completion */ } - } else + } else { g_print (_("Error: %s properties, nor it is a setting name.\n"), tmp_err->message); - g_clear_error (&tmp_err); + } } } } @@ -7661,7 +7664,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t /* Show description for all properties */ print_setting_description (ss); } else { - GError *tmp_err = NULL; + gs_free_error GError *tmp_err = NULL; gs_free char *prop_name = NULL; prop_name = is_property_valid (ss, cmd_arg_p, &tmp_err); @@ -7678,11 +7681,11 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t cmd_arg_p); if (s_tmp) print_setting_description (s_tmp); - else + else { g_print (_("Error: invalid property: %s, " "neither a valid setting name.\n"), tmp_err->message); - g_clear_error (&tmp_err); + } } } } @@ -7769,19 +7772,21 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if ( menu_ctx.curr_setting && (!cmd_arg || strcmp (cmd_arg, "all") != 0)) { - GError *tmp_err = NULL; - (void) nm_setting_verify (menu_ctx.curr_setting, NULL, &tmp_err); + gs_free_error GError *tmp_err = NULL; + + nm_setting_verify (menu_ctx.curr_setting, NULL, &tmp_err); g_print (_("Verify setting '%s': %s\n"), nm_setting_get_name (menu_ctx.curr_setting), tmp_err ? tmp_err->message : "OK"); - g_clear_error (&tmp_err); } else { - GError *tmp_err = NULL; - gboolean valid, modified; + gs_free_error GError *tmp_err = NULL; gboolean fixed = TRUE; + gboolean modified; + gboolean valid; valid = nm_connection_verify (connection, &tmp_err); - if (!valid && (g_strcmp0 (cmd_arg, "fix") == 0)) { + if ( !valid + && nm_streq0 (cmd_arg, "fix")) { /* Try to fix normalizable errors */ g_clear_error (&tmp_err); fixed = nm_connection_normalize (connection, NULL, &modified, &tmp_err); @@ -7790,7 +7795,6 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t tmp_err ? tmp_err->message : "OK"); if (!fixed) g_print (_("The error cannot be fixed automatically.\n")); - g_clear_error (&tmp_err); } break; From ec982ceb8ebc9a2f9c3ee14f89735bce818ee948 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 08:33:32 +0200 Subject: [PATCH 28/74] cli: fix dereferncing NULL pointer in parse_passwords() with empty file Warned by coverity. --- clients/cli/connections.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index c88fbd2abb..b8905cc78a 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -2711,7 +2711,6 @@ parse_passwords (const char *passwd_file, GError **error) if (!passwd_file) return g_steal_pointer (&pwds_hash); - /* Read the passwords file */ if (!g_file_get_contents (passwd_file, &contents, &len, &local_err)) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, _("failed to read passwd-file '%s': %s"), @@ -2721,7 +2720,7 @@ parse_passwords (const char *passwd_file, GError **error) } strv = nm_utils_strsplit_set (contents, "\r\n"); - for (iter = strv; *iter; iter++) { + for (iter = strv; strv && *iter; iter++) { gs_free char *iter_s = g_strdup (*iter); pwd = strchr (iter_s, ':'); From af4a41cc4c1cf17bf9d04477cb6c2ebed5daa6ee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 08:44:12 +0200 Subject: [PATCH 29/74] cli: fix type for loop variable in _get_fcn_vlan_xgress_priority_map() Coverity correctly points out that nm_setting_vlan_get_num_priorities() can return a negative value (-1 on assertion). Handle that by using the right integer type. --- clients/common/nm-meta-setting-desc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index c1a5c2a7fe..5c71868d06 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -3755,7 +3755,7 @@ _get_fcn_vlan_xgress_priority_map (ARGS_GET_FCN) NMVlanPriorityMap map_type = _vlan_priority_map_type_from_property_info (property_info); NMSettingVlan *s_vlan = NM_SETTING_VLAN (setting); GString *str = NULL; - guint32 i, num; + gint32 i, num; RETURN_UNSUPPORTED_GET_TYPE (); From b5793b74ca096990f138475820e519f5a3fc69b2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 08:46:38 +0200 Subject: [PATCH 30/74] libnm: fix assertions in NMSettingVlan's priority API Most of these functions did not ever return failure. The functions were assertin that the input was valid (and then returned a special value). But they did not fail under regular conditions. Fix the gtk-doc for some of these to not claim to be able to fail. For some (like nm_setting_vlan_add_priority_str() and nm_setting_vlan_get_priority()), actually let them fail for valid input (instead of asserting). --- libnm-core/nm-setting-vlan.c | 41 ++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index a7debbf1d7..d69a894621 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -221,7 +221,7 @@ check_replace_duplicate_priority (GSList *list, guint32 from, guint32 to) * the Linux SKB priorities to 802.1p priorities. * * Returns: %TRUE if the entry was successfully added to the list, or it - * overwrote the old value, %FALSE if error + * overwrote the old value, %FALSE if @str is not a valid mapping. */ gboolean nm_setting_vlan_add_priority_str (NMSettingVlan *setting, @@ -235,11 +235,11 @@ nm_setting_vlan_add_priority_str (NMSettingVlan *setting, g_return_val_if_fail (map == NM_VLAN_INGRESS_MAP || map == NM_VLAN_EGRESS_MAP, FALSE); g_return_val_if_fail (str && str[0], FALSE); - list = get_map (setting, map); - item = priority_map_new_from_str (map, str); if (!item) - g_return_val_if_reached (FALSE); + return FALSE; + + list = get_map (setting, map); /* Duplicates get replaced */ if (check_replace_duplicate_priority (list, item->from, item->to)) { @@ -264,7 +264,7 @@ nm_setting_vlan_add_priority_str (NMSettingVlan *setting, * #NMSettingVlan:ingress_priority_map or #NMSettingVlan:egress_priority_map * properties of this setting. * - * Returns: return the number of ingress/egress priority entries, -1 if error + * Returns: return the number of ingress/egress priority entries. **/ gint32 nm_setting_vlan_get_num_priorities (NMSettingVlan *setting, NMVlanPriorityMap map) @@ -280,13 +280,13 @@ nm_setting_vlan_get_num_priorities (NMSettingVlan *setting, NMVlanPriorityMap ma * @setting: the #NMSettingVlan * @map: the type of priority map * @idx: the zero-based index of the ingress/egress priority map entry - * @out_from: (out): on return the value of the priority map's 'from' item - * @out_to: (out): on return the value of priority map's 'to' item + * @out_from: (out) (allow-none): on return the value of the priority map's 'from' item + * @out_to: (out) (allow-none): on return the value of priority map's 'to' item * * Retrieve one of the entries of the #NMSettingVlan:ingress_priority_map * or #NMSettingVlan:egress_priority_map properties of this setting. * - * Returns: %TRUE if a priority map was returned, %FALSE if error + * Returns: returns %TRUE if @idx is in range. Otherwise %FALSE. **/ gboolean nm_setting_vlan_get_priority (NMSettingVlan *setting, @@ -295,21 +295,23 @@ nm_setting_vlan_get_priority (NMSettingVlan *setting, guint32 *out_from, guint32 *out_to) { - GSList *list = NULL; - NMVlanQosMapping *item = NULL; + NMVlanQosMapping *item; + GSList *list; g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE); - g_return_val_if_fail (map == NM_VLAN_INGRESS_MAP || map == NM_VLAN_EGRESS_MAP, FALSE); - g_return_val_if_fail (out_from != NULL, FALSE); - g_return_val_if_fail (out_to != NULL, FALSE); + g_return_val_if_fail (NM_IN_SET (map, NM_VLAN_INGRESS_MAP, NM_VLAN_EGRESS_MAP), FALSE); list = get_map (setting, map); - g_return_val_if_fail (idx < g_slist_length (list), FALSE); - item = g_slist_nth_data (list, idx); - g_assert (item); - *out_from = item->from; - *out_to = item->to; + + if (!item) { + NM_SET_OUT (out_from, 0); + NM_SET_OUT (out_to, 0); + return FALSE; + } + + NM_SET_OUT (out_from, item->from); + NM_SET_OUT (out_to, item->to); return TRUE; } @@ -331,8 +333,7 @@ nm_setting_vlan_get_priority (NMSettingVlan *setting, * If @map is #NM_VLAN_EGRESS_MAP then @from is the Linux SKB priority value and * @to is the outgoing 802.1q VLAN Priority Code Point (PCP) value. * - * Returns: %TRUE if the new priority mapping was successfully added to the - * list, %FALSE if error + * Returns: %TRUE. */ gboolean nm_setting_vlan_add_priority (NMSettingVlan *setting, From f61e274df973b515b17d958ac877fa78a3a2c2b0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 08:52:26 +0200 Subject: [PATCH 31/74] libnm: try to avoid coverity warning in assertion() Coverity thinks that this could be NULL sometimes. Try to check for that to shut up the warning. --- libnm-core/nm-setting-vpn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c index 56098392b5..fc9c5184da 100644 --- a/libnm-core/nm-setting-vpn.c +++ b/libnm-core/nm-setting-vpn.c @@ -277,7 +277,7 @@ foreach_item_helper (NMSettingVpn *self, } for (i = 0; i < len; i++) { - nm_assert (keys[i]); + nm_assert (keys && keys[i]); keys[i] = g_strdup (keys[i]); } nm_assert (!keys[i]); From 186d559d6320f159988a7155f9049c714510c6be Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 08:55:30 +0200 Subject: [PATCH 32/74] libnm: avoid NM_CONST_MAX() in enum definition for NMTeamAttribute This confuses coverity. Just use MAX(). MAX() is usually not preferred as it evaluates the arguments more than once. But in this case, it is of course fine. CID 202433 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)1. expr_not_constant: expression must have a constant value --- libnm-core/nm-team-utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-team-utils.h b/libnm-core/nm-team-utils.h index 7da42a3f43..c4631bec9a 100644 --- a/libnm-core/nm-team-utils.h +++ b/libnm-core/nm-team-utils.h @@ -61,7 +61,7 @@ typedef enum { NM_TEAM_ATTRIBUTE_PORT_LACP_KEY, _NM_TEAM_ATTRIBUTE_PORT_NUM, - _NM_TEAM_ATTRIBUTE_NUM = NM_CONST_MAX (_NM_TEAM_ATTRIBUTE_MASTER_NUM, _NM_TEAM_ATTRIBUTE_PORT_NUM), + _NM_TEAM_ATTRIBUTE_NUM = MAX (_NM_TEAM_ATTRIBUTE_MASTER_NUM, _NM_TEAM_ATTRIBUTE_PORT_NUM), } NMTeamAttribute; From 8988a12ade61c23d7442076495e4c6492aa8ccc1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:01:32 +0200 Subject: [PATCH 33/74] core: assert for valid arguments in sort_captured_addresses() and _addresses_sort_cmp() Coverity thinks that the arguments could be %NULL. Add an assertion, hoping to silence coverity. --- src/nm-ip4-config.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 86c8292506..36e75bb2d2 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -544,6 +544,9 @@ _addresses_sort_cmp (gconstpointer a, gconstpointer b, gpointer user_data) const NMPlatformIP4Address *a2 = NMP_OBJECT_CAST_IP4_ADDRESS (*((const NMPObject **) b)); guint32 n1, n2; + nm_assert (a1); + nm_assert (a2); + /* Sort by address type. For example link local will * be sorted *after* a global address. */ p1 = _addresses_sort_cmp_get_prio (a1->address); @@ -577,6 +580,9 @@ sort_captured_addresses (const CList *lst_a, const CList *lst_b, gconstpointer u const NMPlatformIP4Address *addr_a = NMP_OBJECT_CAST_IP4_ADDRESS (c_list_entry (lst_a, NMDedupMultiEntry, lst_entries)->obj); const NMPlatformIP4Address *addr_b = NMP_OBJECT_CAST_IP4_ADDRESS (c_list_entry (lst_b, NMDedupMultiEntry, lst_entries)->obj); + nm_assert (addr_a); + nm_assert (addr_b); + /* Primary addresses first */ return NM_FLAGS_HAS (addr_a->n_ifa_flags, IFA_F_SECONDARY) - NM_FLAGS_HAS (addr_b->n_ifa_flags, IFA_F_SECONDARY); From 5b9a848a82a756bfa01799a4f32d52af1490ec94 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:09:45 +0200 Subject: [PATCH 34/74] device/adsl: restore brfd value on error in br2684_assign_vcc() Warned by coverity: we assert above that brfd is -1, so we must always restore it to -1 in the error case. Technically, not a problem because socket() is documented to return only -1 on error already. Apparently coverity does not believe that. --- src/devices/adsl/nm-device-adsl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 59c87851d1..536d7cccb7 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -155,6 +155,7 @@ br2684_assign_vcc (NMDeviceAdsl *self, NMSettingAdsl *s_adsl) if (priv->brfd < 0) { errsv = errno; _LOGE (LOGD_ADSL, "failed to open ATM control socket (%d)", errsv); + priv->brfd = -1; return FALSE; } From 458a2edbb24061bfd9532594e447da2e23819092 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:16:34 +0200 Subject: [PATCH 35/74] device/wireguard: fix explicit_bzero() call on peers buffer in link_config() Correctly warned by coverity. --- src/devices/nm-device-wireguard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index f4da539356..d08f1afd47 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -1467,7 +1467,7 @@ link_config (NMDeviceWireGuard *self, plpeers_len, wg_change_flags); - nm_explicit_bzero (plpeers, sizeof (plpeers) * plpeers_len); + nm_explicit_bzero (plpeers, sizeof (plpeers[0]) * plpeers_len); if (r < 0) { NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED); From e001424ae280ac71d23f5ca40787a190fd35f612 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:19:03 +0200 Subject: [PATCH 36/74] libnm/keyfile: silence "Identical code for different branches" complaint in _read_setting_wireguard_peer() That both branches of the "if" do the same, looks suspicious to Coverity. Work around it by not doing it. --- libnm-core/nm-keyfile.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 85a9ea11cb..b0c7a13478 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -3394,10 +3394,9 @@ _read_setting_wireguard_peer (KeyfileReaderInfo *info) return; if (!nm_wireguard_peer_is_valid (peer, TRUE, TRUE, &error)) { - if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, - _("peer '%s' is invalid: %s"), - info->group, error->message)) - return; + handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, + _("peer '%s' is invalid: %s"), + info->group, error->message); return; } From 4596d7793cee22228e68d102fa199ed1736a2050 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:24:01 +0200 Subject: [PATCH 37/74] initrd: avoid coverity warning in parse_ip() about "Dereference before null check" get_word() only moves the "argument" pointer forward. It never sets it to %NULL. Also, above we already dereference argument, so Coverity thinks that this NULL check indicates a bug. Drop it to silence Coverity. --- src/initrd/nmi-cmdline-reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c index ccdd1f2992..7a3af8d661 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -244,7 +244,7 @@ parse_ip (GHashTable *connections, const char *sysfs_dir, char *argument) dns[0] = tmp; dns[1] = get_word (&argument, ':'); dns_addr_family[1] = guess_ip_address_family (dns[1]); - if (argument && *argument) + if (*argument) _LOGW (LOGD_CORE, "Ignoring extra: '%s'.", argument); } else { mtu = tmp; From e6fa3ce2dfd45ef3556b31b8f51c88a78c4e4ce4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:27:52 +0200 Subject: [PATCH 38/74] shared: explicitly ignore return value of g_utf8_validate() Coverity doesn't like us ignoring the return value, although we really only care about the "p" output pointer. Try casting the result to (void), maybe that silences Coverity. --- shared/nm-glib-aux/nm-shared-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index c8a253a668..b2ec547092 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -1939,7 +1939,7 @@ nm_utils_buf_utf8safe_escape (gconstpointer buf, gssize buflen, NMUtilsStrUtf8Sa break; s = &p[1]; - g_utf8_validate (s, buflen, &p); + (void) g_utf8_validate (s, buflen, &p); } while (TRUE); *to_free = g_string_free (gstr, FALSE); From d76df4c1398324170f306fc3f28b7910d6972ccc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:30:33 +0200 Subject: [PATCH 39/74] shared: try avoid coverity warning in _nm_utils_user_data_unpack() Coverity says CID 202453 (#1 of 1): Wrong sizeof argument (SIZEOF_MISMATCH)suspicious_sizeof: Passing argument user_data of type gconstpointer and argument (gsize)nargs * 8UL /* sizeof (gconstpointer) */ to function g_slice_free1 is suspicious. Let's pass instead the "data" pointer. It's the same, but maybe that avoids the warning. --- shared/nm-glib-aux/nm-shared-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index b2ec547092..33749d3a1c 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -2878,7 +2878,7 @@ _nm_utils_user_data_unpack (gpointer user_data, int nargs, ...) } va_end (ap); - g_slice_free1 (((gsize) nargs) * sizeof (gconstpointer), user_data); + g_slice_free1 (((gsize) nargs) * sizeof (gconstpointer), data); } /*****************************************************************************/ From 8fb954b81d6f6ebc5f0b4fda3efa6f02f4c06e8a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 09:37:46 +0200 Subject: [PATCH 40/74] shared: refactor nm_utils_g_slist_strlist_cmp() to avoid dead-code warning from Coverity Coverity sees that "return 0" cannot be reached. Refactor the code, to avoid the warning. --- shared/nm-glib-aux/nm-shared-utils.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 33749d3a1c..9bfd5ae2b8 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -2834,10 +2834,15 @@ nm_utils_g_slist_find_str (const GSList *list, int nm_utils_g_slist_strlist_cmp (const GSList *a, const GSList *b) { - for (; a && b; a = a->next, b = b->next) + while (TRUE) { + if (!a) + return !b ? 0 : -1; + if (!b) + return 1; NM_CMP_DIRECT_STRCMP0 (a->data, b->data); - NM_CMP_SELF (a, b); - return 0; + a = a->next; + b = b->next; + } } /*****************************************************************************/ From c8cee413dddd4a6bb45beb601b93c201ee1b5be1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 11:26:25 +0200 Subject: [PATCH 41/74] n-acd: fix leaking socket handle in n_acd_socket_new() when setsockopt() fails Found by Coverity. --- shared/n-acd/src/n-acd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shared/n-acd/src/n-acd.c b/shared/n-acd/src/n-acd.c index a0a48c5889..d434d1d14e 100644 --- a/shared/n-acd/src/n-acd.c +++ b/shared/n-acd/src/n-acd.c @@ -127,8 +127,10 @@ static int n_acd_socket_new(int *fdp, int fd_bpf_prog, NAcdConfig *config) { if (fd_bpf_prog >= 0) { r = setsockopt(s, SOL_SOCKET, SO_ATTACH_BPF, &fd_bpf_prog, sizeof(fd_bpf_prog)); - if (r < 0) - return -c_errno(); + if (r < 0) { + r = -c_errno(); + goto error; + } } r = bind(s, (struct sockaddr *)&address, sizeof(address)); From 9e7ca3e0913e2ba6a38dd5b0cbd3a7b3e34cd286 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Jul 2019 11:52:53 +0200 Subject: [PATCH 42/74] n-dhcp4: avoid "-Werror=declaration-after-statement" warning with static_assert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we build n-dhcp4 for NetworkManager we get a compiler warning. This can also be reproduced by building n-dhcp4 alone: $ CFLAGS='-Werror=declaration-after-statement' meson build && ninja -C build ... [36/47] Compiling C object 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o'. FAILED: src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o ccache cc -Isrc/25a6634@@ndhcp4-private@sta -Isrc -I../src -Isubprojects/c-list/src -I../subprojects/c-list/src -Isubprojects/c-siphash/src -I../subprojects/c-siphash/src -Isubprojects/c-stdaux/src -I../subprojects/c-stdaux/src -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c11 -g -D_GNU_SOURCE -Werror=declaration-after-statement -fPIC -fvisibility=hidden -fno-common -MD -MQ 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o' -MF 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o.d' -o 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o' -c ../src/n-dhcp4-outgoing.c ../src/n-dhcp4-outgoing.c: In function ‘n_dhcp4_outgoing_new’: ../src/n-dhcp4-outgoing.c:63:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] 63 | static_assert(N_DHCP4_NETWORK_IP_MINIMUM_MAX_SIZE >= N_DHCP4_OUTGOING_MAX_PHDR + | ^~~~~~~~~~~~~ --- shared/n-dhcp4/src/n-dhcp4-outgoing.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-outgoing.c b/shared/n-dhcp4/src/n-dhcp4-outgoing.c index c44b5880aa..9912330886 100644 --- a/shared/n-dhcp4/src/n-dhcp4-outgoing.c +++ b/shared/n-dhcp4/src/n-dhcp4-outgoing.c @@ -54,8 +54,6 @@ int n_dhcp4_outgoing_new(NDhcp4Outgoing **outgoingp, size_t max_size, uint8_t overload) { _c_cleanup_(n_dhcp4_outgoing_freep) NDhcp4Outgoing *outgoing = NULL; - c_assert(!(overload & ~(N_DHCP4_OVERLOAD_FILE | N_DHCP4_OVERLOAD_SNAME))); - /* * Make sure the minimum limit is bigger than the maximum protocol * header plus the DHCP-message-header plus a single OPTION_END byte. @@ -64,6 +62,8 @@ int n_dhcp4_outgoing_new(NDhcp4Outgoing **outgoingp, size_t max_size, uint8_t ov sizeof(NDhcp4Message) + 1, "Invalid minimum IP packet limit"); + c_assert(!(overload & ~(N_DHCP4_OVERLOAD_FILE | N_DHCP4_OVERLOAD_SNAME))); + outgoing = calloc(1, sizeof(*outgoing)); if (!outgoing) return -ENOMEM; From 9f4302e1320081088d29db1f98c39fc4545267e3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 11:29:42 +0200 Subject: [PATCH 43/74] libnm: fix parsing invalid "pvid" attribute in GVariant in _nm_utils_bridge_vlans_from_dbus() Complained by Coverity. --- libnm-core/nm-utils.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 55b004c816..c16c9004e7 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -5989,10 +5989,12 @@ _nm_utils_bridge_vlans_from_dbus (NMSetting *setting, if (vid_start > vid_end) continue; - g_variant_lookup (vlan_var, "pvid", "b", &pvid); + if (!g_variant_lookup (vlan_var, "pvid", "b", &pvid)) + pvid = FALSE; if (pvid && vid_start != vid_end) continue; - g_variant_lookup (vlan_var, "untagged", "b", &untagged); + if (!g_variant_lookup (vlan_var, "untagged", "b", &untagged)) + untagged = FALSE; vlan = nm_bridge_vlan_new (vid_start, vid_end); nm_bridge_vlan_set_untagged (vlan, untagged); From 23fa1b3272f5af3520c619f2d4101dcc2fbbf914 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 11:43:54 +0200 Subject: [PATCH 44/74] adsl: avoid coverity false-positive when using strcpy() for interface name CID 59391 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 31. fixed_size_dest: You might overrun the 16-character fixed-size string be.ifspec.spec.ifname by copying priv->nas_ifname without checking the length. --- src/devices/adsl/nm-device-adsl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 536d7cccb7..34efdb0156 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -207,7 +207,7 @@ br2684_assign_vcc (NMDeviceAdsl *self, NMSettingAdsl *s_adsl) memset (&be, 0, sizeof (be)); be.backend_num = ATM_BACKEND_BR2684; be.ifspec.method = BR2684_FIND_BYIFNAME; - strcpy (be.ifspec.spec.ifname, priv->nas_ifname); + nm_utils_ifname_cpy (be.ifspec.spec.ifname, priv->nas_ifname); be.fcs_in = BR2684_FCSIN_NO; be.fcs_out = BR2684_FCSOUT_NO; be.encaps = is_llc ? BR2684_ENCAPS_LLC : BR2684_ENCAPS_VC; From 483de2bb93ff66afc439f89d3ce8f539acd87867 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 13:34:40 +0200 Subject: [PATCH 45/74] platform/tests: relax assertion for platform signal in test_slave() Seen on gitlab-ci. NMPlatformSignalAssert: ../src/platform/tests/test-link.c:260, test_slave(): failure to accept signal [0,2] times: link-changed-changed ifindex 15 (3 times received) ERROR: src/platform/tests/test-link-linux - too few tests run (expected 76, got 6) ERROR: src/platform/tests/test-link-linux - exited with status 133 (terminated by signal 5?) --- src/platform/tests/test-link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 5926dac112..27ec3f0706 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -257,7 +257,7 @@ test_slave (int master, int type, SignalData *master_changed) case NM_LINK_TYPE_TEAM: g_assert (nm_platform_link_set_down (NM_PLATFORM_GET, ifindex)); accept_signal (link_changed); - accept_signals (master_changed, 0, 2); + accept_signals (master_changed, 0, 3); break; default: break; From a32976568c13e22dd560610374782250f4d4ba9e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Aug 2019 15:16:31 +0200 Subject: [PATCH 46/74] n-dhcp4: remove dead code Reported by coverity. --- shared/n-dhcp4/src/n-dhcp4-c-lease.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-lease.c b/shared/n-dhcp4/src/n-dhcp4-c-lease.c index db1ffbae82..208ad92a6a 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-lease.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-lease.c @@ -44,10 +44,7 @@ static int n_dhcp4_incoming_get_timeouts(NDhcp4Incoming *message, uint64_t *t1p, } else if (u32 == UINT32_MAX) { lifetime = UINT64_MAX; } else { - if (u32 == UINT32_MAX) - lifetime = UINT64_MAX; - else - lifetime = u32 * (1000000000ULL); + lifetime = u32 * (1000000000ULL); } r = n_dhcp4_incoming_query_t2(message, &u32); From 50ae9c936cd5d0838c68123655feb05c1267e89e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Aug 2019 15:25:13 +0200 Subject: [PATCH 47/74] tui: newt: remove NULL checks after dereference priv->start_buttons and priv->end_buttons are initialized at construction and never changed and so the checks are not needed. --- clients/tui/newt/nmt-newt-button-box.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clients/tui/newt/nmt-newt-button-box.c b/clients/tui/newt/nmt-newt-button-box.c index 79222c9ae4..c69c191152 100644 --- a/clients/tui/newt/nmt-newt-button-box.c +++ b/clients/tui/newt/nmt-newt-button-box.c @@ -262,12 +262,10 @@ nmt_newt_button_box_size_request (NmtNewtWidget *widget, size_request_buttons (bbox, priv->start_buttons, width, height); size_request_buttons (bbox, priv->end_buttons, width, height); - if (priv->start_buttons && priv->end_buttons) { - if (priv->orientation == NMT_NEWT_BUTTON_BOX_HORIZONTAL) - *width += 1; - else - *height += 1; - } + if (priv->orientation == NMT_NEWT_BUTTON_BOX_HORIZONTAL) + *width += 1; + else + *height += 1; } static void From 1b30797bc16c7e0cc2e6c5686e52aa12ef5a5425 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 17:13:15 +0200 Subject: [PATCH 48/74] core: assert for valid arguments in _addresses_sort_cmp() Coverity thinks that the arguments could be %NULL. Add an assertion, hoping to silence coverity. --- src/nm-ip6-config.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 1096fb4246..1810d51185 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -318,6 +318,9 @@ sort_captured_addresses (const CList *lst_a, const CList *lst_b, gconstpointer u const NMPlatformIP6Address *addr_a = NMP_OBJECT_CAST_IP6_ADDRESS (c_list_entry (lst_a, NMDedupMultiEntry, lst_entries)->obj); const NMPlatformIP6Address *addr_b = NMP_OBJECT_CAST_IP6_ADDRESS (c_list_entry (lst_b, NMDedupMultiEntry, lst_entries)->obj); + nm_assert (addr_a); + nm_assert (addr_b); + return _addresses_sort_cmp (addr_a, addr_b, ((NMSettingIP6ConfigPrivacy) GPOINTER_TO_INT (user_data)) == NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_TEMP_ADDR); } From 88bcf87ad94ed039f4ff8cd72676a0b3725cf950 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 15:40:05 +0200 Subject: [PATCH 49/74] device: trigger a connectivity check when device disconnects https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/219 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/225 --- src/devices/nm-device.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index cf80774298..f07c4f09a5 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3186,14 +3186,11 @@ nm_device_check_connectivity (NMDevice *self, NMDeviceConnectivityCallback callback, gpointer user_data) { - NMDeviceConnectivityHandle *handle; - if (!concheck_is_possible (self)) return NULL; concheck_periodic_schedule_set (self, addr_family, CONCHECK_SCHEDULE_CHECK_EXTERNAL); - handle = concheck_start (self, addr_family, callback, user_data, FALSE); - return handle; + return concheck_start (self, addr_family, callback, user_data, FALSE); } void @@ -15117,6 +15114,7 @@ _set_state_full (NMDevice *self, gboolean no_firmware = FALSE; NMSettingsConnection *sett_conn; NMSettingSriov *s_sriov; + gboolean concheck_now; g_return_if_fail (NM_IS_DEVICE (self)); @@ -15452,8 +15450,11 @@ _set_state_full (NMDevice *self, if (ip_config_valid (old_state) && !ip_config_valid (state)) notify_ip_properties (self); - concheck_update_interval (self, AF_INET, state == NM_DEVICE_STATE_ACTIVATED); - concheck_update_interval (self, AF_INET6, state == NM_DEVICE_STATE_ACTIVATED); + concheck_now = NM_IN_SET (state, NM_DEVICE_STATE_ACTIVATED, + NM_DEVICE_STATE_DISCONNECTED) + || old_state >= NM_DEVICE_STATE_ACTIVATED; + concheck_update_interval (self, AF_INET, concheck_now); + concheck_update_interval (self, AF_INET6, concheck_now); /* Dispose of the cached activation request */ if (req) From 643bc4ca2275ed00ed6607d2b308f65d531675b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 17:45:49 +0200 Subject: [PATCH 50/74] libnm: remove dead code in nm_team_setting_config_get() I was aware that this code is not reachable. But for consistency, it seems better to be explict about it (to avoid future bugs when refactoring). Anyway, Coverity complains about it. So assert instead. --- libnm-core/nm-team-utils.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libnm-core/nm-team-utils.c b/libnm-core/nm-team-utils.c index ac834d7408..63018b09f7 100644 --- a/libnm-core/nm-team-utils.c +++ b/libnm-core/nm-team-utils.c @@ -1587,8 +1587,8 @@ nm_team_setting_config_get (const NMTeamSetting *self) || _team_setting_has_fields_any_v (self, attr_lst_runner_pt3, G_N_ELEMENTS (attr_lst_runner_pt3))) { gboolean list_is_empty2 = TRUE; - if (!list_is_empty) - nm_json_aux_gstr_append_delimiter (gstr); + nm_assert (list_is_empty); + nm_json_aux_gstr_append_obj_name (gstr, "runner", '{'); if (_team_setting_fields_to_json_maybe (self, gstr, !list_is_empty2, attr_lst_runner_pt1, G_N_ELEMENTS (attr_lst_runner_pt1))) From 526601e4f328837fdccb7447ceac0243616d5e2c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 17:16:00 +0200 Subject: [PATCH 51/74] device/bluetooth: explicitly ignore return value of ioctl() in nm_bluez5_dun_cleanup() Coverity doesn't like us not checking the result. --- src/devices/bluetooth/nm-bluez5-dun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/bluetooth/nm-bluez5-dun.c b/src/devices/bluetooth/nm-bluez5-dun.c index 04859f9ac7..b9a1fa0ab8 100644 --- a/src/devices/bluetooth/nm-bluez5-dun.c +++ b/src/devices/bluetooth/nm-bluez5-dun.c @@ -386,7 +386,7 @@ nm_bluez5_dun_cleanup (NMBluez5DunContext *context) struct rfcomm_dev_req req = { 0 }; req.dev_id = context->rfcomm_id; - ioctl (context->rfcomm_fd, RFCOMMRELEASEDEV, &req); + (void) ioctl (context->rfcomm_fd, RFCOMMRELEASEDEV, &req); context->rfcomm_id = -1; } nm_close (context->rfcomm_fd); From 9ed26de3daf71020dd0148fcc1ad9de8cc5e5009 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 17:25:58 +0200 Subject: [PATCH 52/74] supplicant: fix nm_supplicant_settings_verify_setting() honoring the string length We must not just pretend that the value is a NULL terminated string. That's why we have the length argument. --- .../nm-supplicant-settings-verify.c | 50 +++++++++++-------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/src/supplicant/nm-supplicant-settings-verify.c b/src/supplicant/nm-supplicant-settings-verify.c index 1bd71320a4..05dcf2c80e 100644 --- a/src/supplicant/nm-supplicant-settings-verify.c +++ b/src/supplicant/nm-supplicant-settings-verify.c @@ -206,10 +206,7 @@ validate_type_keyword (const struct Opt * opt, const char * value, const guint32 len) { - char **allowed; - char **candidates = NULL; - char **candidate; - gboolean found = FALSE; + gs_free char *value_free = NULL; g_return_val_if_fail (opt != NULL, FALSE); g_return_val_if_fail (value != NULL, FALSE); @@ -218,26 +215,33 @@ validate_type_keyword (const struct Opt * opt, if (!opt->str_allowed) return TRUE; - candidates = g_strsplit (value, " ", 0); - if (!candidates) - goto out; + value = nm_strndup_a (300, value, len, &value_free); /* validate each space-separated word in 'value' */ - for (candidate = candidates; *candidate; candidate++) { - found = FALSE; - for (allowed = (char **) opt->str_allowed; *allowed; allowed++) { - if (strcmp (*candidate, *allowed) == 0) { - found = TRUE; - break; - } - } - if (!found) - break; - } -out: - g_strfreev (candidates); - return found; + while (TRUE) { + char *s; + + while (value[0] == ' ') + value++; + + if (value[0] == '\0') + return TRUE; + + s = strchr (value, ' '); + if (s) { + s[0] = '\0'; + s++; + } + + if (nm_utils_strv_find_first ((char **) opt->str_allowed, -1, value) < 0) + return FALSE; + + if (!s) + return TRUE; + + value = s; + } } OptType @@ -254,7 +258,9 @@ nm_supplicant_settings_verify_setting (const char * key, g_return_val_if_fail (value != NULL, FALSE); if (strcmp (key, "mode") == 0) { - if (strcmp (value, "1") && strcmp (value, "2") && strcmp (value, "5")) + if (len != 1) + return TYPE_INVALID; + if (!NM_IN_SET (value[0], '1', '2', '5')) return TYPE_INVALID; return TYPE_INT; } From 55143dad95dde9e5eae2a5084e39313fcfba4a4a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 17:34:59 +0200 Subject: [PATCH 53/74] supplicant: don't put binary data in error message for supplicant For better or worse, the API does not require the value to be a UTF-8 string. We cannot just concatenate binary to a string. Instead, backslash escape it with utf8safe-escape. Also, this will shut up a (wrong) coverity warning at this place. --- src/supplicant/nm-supplicant-config.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/supplicant/nm-supplicant-config.c b/src/supplicant/nm-supplicant-config.c index 806c087c8a..f6e3c7da53 100644 --- a/src/supplicant/nm-supplicant-config.c +++ b/src/supplicant/nm-supplicant-config.c @@ -139,11 +139,17 @@ nm_supplicant_config_add_option_with_type (NMSupplicantConfig *self, else { type = nm_supplicant_settings_verify_setting (key, value, len); if (type == TYPE_INVALID) { - char buf[255]; - memset (&buf[0], 0, sizeof (buf)); - memcpy (&buf[0], value, len > 254 ? 254 : len); + gs_free char *str_free = NULL; + const char *str; + + str = nm_utils_buf_utf8safe_escape (value, len, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL, &str_free); + + str = nm_strquote_a (255, str); + g_set_error (error, NM_SUPPLICANT_ERROR, NM_SUPPLICANT_ERROR_CONFIG, - "key '%s' and/or value '%s' invalid", key, hidden ?: buf); + "key '%s' and/or value %s invalid", + key, + hidden ?: str); return FALSE; } } From 722b167953c2384615cb11bbc88988dec7e647ab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 2 Aug 2019 18:05:18 +0200 Subject: [PATCH 54/74] supplicant: mark static arrays as const and static in "nm-supplicant-settings-verify.c" They should be "static" and only visible to this source file. Also, they should be "const", that allows the linker to place them into read-only memory. --- .../nm-supplicant-settings-verify.c | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/supplicant/nm-supplicant-settings-verify.c b/src/supplicant/nm-supplicant-settings-verify.c index 05dcf2c80e..b37fae9ed4 100644 --- a/src/supplicant/nm-supplicant-settings-verify.c +++ b/src/supplicant/nm-supplicant-settings-verify.c @@ -30,7 +30,7 @@ struct Opt { const gint32 int_low; /* Inclusive */ const gint32 int_high; /* Inclusive; max length for strings */ const gboolean str_allowed_multiple; - const char ** str_allowed; + const char *const*str_allowed; }; static gboolean validate_type_int (const struct Opt * opt, @@ -63,31 +63,31 @@ static const struct validate_entry validate_table[] = { { TYPE_KEYWORD, validate_type_keyword }, }; -const char * pairwise_allowed[] = { "CCMP", "TKIP", "NONE", NULL }; -const char * group_allowed[] = { "CCMP", "TKIP", "WEP104", "WEP40", NULL }; -const char * proto_allowed[] = { "WPA", "RSN", NULL }; -const char * key_mgmt_allowed[] = { "WPA-PSK", "WPA-PSK-SHA256", "FT-PSK", - "WPA-EAP", "WPA-EAP-SHA256", "FT-EAP", "FT-EAP-SHA384", - "FILS-SHA256", "FILS-SHA384", - "IEEE8021X", "WPA-NONE", "SAE", - "NONE", NULL }; -const char * auth_alg_allowed[] = { "OPEN", "SHARED", "LEAP", NULL }; -const char * eap_allowed[] = { "LEAP", "MD5", "TLS", "PEAP", "TTLS", "SIM", - "PSK", "FAST", "PWD", NULL }; +static const char *const pairwise_allowed[] = { "CCMP", "TKIP", "NONE", NULL }; +static const char *const group_allowed[] = { "CCMP", "TKIP", "WEP104", "WEP40", NULL }; +static const char *const proto_allowed[] = { "WPA", "RSN", NULL }; +static const char *const key_mgmt_allowed[] = { "WPA-PSK", "WPA-PSK-SHA256", "FT-PSK", + "WPA-EAP", "WPA-EAP-SHA256", "FT-EAP", "FT-EAP-SHA384", + "FILS-SHA256", "FILS-SHA384", + "IEEE8021X", "WPA-NONE", "SAE", + "NONE", NULL }; +static const char *const auth_alg_allowed[] = { "OPEN", "SHARED", "LEAP", NULL }; +static const char *const eap_allowed[] = { "LEAP", "MD5", "TLS", "PEAP", "TTLS", "SIM", + "PSK", "FAST", "PWD", NULL }; -const char * phase1_allowed[] = {"peapver=0", "peapver=1", "peaplabel=1", - "peap_outer_success=0", "include_tls_length=1", - "sim_min_num_chal=3", "fast_provisioning=0", - "fast_provisioning=1", "fast_provisioning=2", - "fast_provisioning=3", "tls_disable_tlsv1_0=0", - "tls_disable_tlsv1_0=1", "tls_disable_tlsv1_1=0", - "tls_disable_tlsv1_1=1", "tls_disable_tlsv1_2=0", - "tls_disable_tlsv1_2=1", NULL }; -const char * phase2_allowed[] = {"auth=PAP", "auth=CHAP", "auth=MSCHAP", - "auth=MSCHAPV2", "auth=GTC", "auth=OTP", - "auth=MD5", "auth=TLS", "autheap=MD5", - "autheap=MSCHAPV2", "autheap=OTP", - "autheap=GTC", "autheap=TLS", NULL }; +static const char *const phase1_allowed[] = { "peapver=0", "peapver=1", "peaplabel=1", + "peap_outer_success=0", "include_tls_length=1", + "sim_min_num_chal=3", "fast_provisioning=0", + "fast_provisioning=1", "fast_provisioning=2", + "fast_provisioning=3", "tls_disable_tlsv1_0=0", + "tls_disable_tlsv1_0=1", "tls_disable_tlsv1_1=0", + "tls_disable_tlsv1_1=1", "tls_disable_tlsv1_2=0", + "tls_disable_tlsv1_2=1", NULL }; +static const char *const phase2_allowed[] = { "auth=PAP", "auth=CHAP", "auth=MSCHAP", + "auth=MSCHAPV2", "auth=GTC", "auth=OTP", + "auth=MD5", "auth=TLS", "autheap=MD5", + "autheap=MSCHAPV2", "autheap=OTP", + "autheap=GTC", "autheap=TLS", NULL }; static const struct Opt opt_table[] = { { "ssid", TYPE_BYTES, 0, 32,FALSE, NULL }, From cc3681c96cdec23dd52cbc653cf0a00c6a289101 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 12:52:03 +0200 Subject: [PATCH 55/74] cli: (trivial) rename do_device_wifi_connect_network() to do_device_wifi_connect() This is consistent with the surrounding code and avoids lines unnecessarily too wide. --- clients/cli/devices.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index b37b40e08a..8abf9ad61a 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -3119,7 +3119,7 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv) } static NMCResultCode -do_device_wifi_connect_network (NmCli *nmc, int argc, char **argv) +do_device_wifi_connect (NmCli *nmc, int argc, char **argv) { NMDevice *device = NULL; NMAccessPoint *ap = NULL; @@ -4020,11 +4020,11 @@ finish: } static NMCCommand device_wifi_cmds[] = { - { "list", do_device_wifi_list, NULL, TRUE, TRUE }, - { "connect", do_device_wifi_connect_network, NULL, TRUE, TRUE }, - { "hotspot", do_device_wifi_hotspot, NULL, TRUE, TRUE }, - { "rescan", do_device_wifi_rescan, NULL, TRUE, TRUE }, - { NULL, do_device_wifi_list, NULL, TRUE, TRUE }, + { "list", do_device_wifi_list, NULL, TRUE, TRUE }, + { "connect", do_device_wifi_connect, NULL, TRUE, TRUE }, + { "hotspot", do_device_wifi_hotspot, NULL, TRUE, TRUE }, + { "rescan", do_device_wifi_rescan, NULL, TRUE, TRUE }, + { NULL, do_device_wifi_list, NULL, TRUE, TRUE }, }; static NMCResultCode From 69ed759ddf4a97723668fefda5ba458792d67b6b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 13:01:37 +0200 Subject: [PATCH 56/74] cli: remove an unnecessary condition Do not check for password when creating a simple connection object for "nmcli dev wifi connect". This makes no difference in practice. The password is checked for existence later on and the connection instance is created anyway. This just makes things look a bit more consistent. --- clients/cli/devices.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 8abf9ad61a..d36382f85c 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -3410,7 +3410,7 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) if (!existing_con) { /* If there are some connection data from user, create a connection and * fill them into proper settings. */ - if (con_name || private || bssid2_arr || password || hidden) + if (con_name || private || bssid2_arr || hidden) connection = nm_simple_connection_new (); if (con_name || private) { From bc614783f032deb166d531161f8eec0ac64f5cc4 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 14:09:40 +0200 Subject: [PATCH 57/74] cli: take a reference to device in AddAndActivateInfo The device could vanish in between. --- clients/cli/devices.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index d36382f85c..f2625eb68f 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1840,6 +1840,13 @@ typedef struct { gboolean create; } AddAndActivateInfo; +static void +add_and_activate_info_free (AddAndActivateInfo *info) +{ + g_object_unref (info->device); + g_free (info); +} + static void add_and_activate_cb (GObject *client, GAsyncResult *result, @@ -1887,7 +1894,7 @@ add_and_activate_cb (GObject *client, } } - g_free (info); + add_and_activate_info_free (info); } static void @@ -1946,7 +1953,7 @@ connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; g_object_unref (active); quit (); - g_free (info); + add_and_activate_info_free (info); return; } @@ -1973,7 +1980,7 @@ connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) g_timeout_add_seconds (nmc->timeout, timeout_cb, nmc); } } - g_free (info); + add_and_activate_info_free (info); } static NMCResultCode @@ -2020,7 +2027,7 @@ do_device_connect (NmCli *nmc, int argc, char **argv) info = g_malloc0 (sizeof (AddAndActivateInfo)); info->nmc = nmc; - info->device = device; + info->device = g_object_ref (device); info->hotspot = FALSE; nm_client_activate_connection_async (nmc->client, @@ -3524,7 +3531,7 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) info = g_malloc0 (sizeof (AddAndActivateInfo)); info->nmc = nmc; - info->device = device; + info->device = g_object_ref (device); info->hotspot = FALSE; info->create = !existing_con; if (existing_con) { @@ -3894,7 +3901,7 @@ do_device_wifi_hotspot (NmCli *nmc, int argc, char **argv) info = g_malloc0 (sizeof (AddAndActivateInfo)); info->nmc = nmc; - info->device = device; + info->device = g_object_ref (device); info->hotspot = TRUE; info->create = TRUE; From a4740fb82ad13fde27801ab31dbbeac51f3a5a75 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 30 Jul 2019 14:14:51 +0200 Subject: [PATCH 58/74] cli: update the existing connection on "dev wifi connect" If we find a matching connection, ensure it's exactly as we want it before actually proceeding to activate it. Fixes this problem: # nmcli dev wifi connect "Network of Doom" password santa <-- bad Error: Connection activation failed: (7) Invalid secrets # nmcli dev wifi connect "Network of Doom" password satan <-- correct Error: Connection activation failed: (7) Invalid secrets The password is now correct, but nmcli chose to re-activate the wrong connection it created previously. --- clients/cli/devices.c | 65 ++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index f2625eb68f..1e199d8af7 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1838,12 +1838,14 @@ typedef struct { NMDevice *device; gboolean hotspot; gboolean create; + char *specific_object; } AddAndActivateInfo; static void add_and_activate_info_free (AddAndActivateInfo *info) { g_object_unref (info->device); + g_free (info->specific_object); g_free (info); } @@ -3125,6 +3127,35 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv) return nmc->return_value; } +static void +activate_update2_cb (GObject *source_object, GAsyncResult *res, gpointer user_data) +{ + NMRemoteConnection *remote_con = NM_REMOTE_CONNECTION (source_object); + AddAndActivateInfo *info = (AddAndActivateInfo *) user_data; + NmCli *nmc = info->nmc; + gs_unref_variant GVariant *ret = NULL; + GError *error = NULL; + + ret = nm_remote_connection_update2_finish (remote_con, res, &error); + + if (!ret) { + g_string_printf (nmc->return_text, _("Error: %s."), error->message); + nmc->return_value = NMC_RESULT_ERROR_UNKNOWN; + g_error_free (error); + quit (); + add_and_activate_info_free (info); + return; + } + + nm_client_activate_connection_async (nmc->client, + NM_CONNECTION (remote_con), + info->device, + info->specific_object, + NULL, + add_and_activate_cb, + info); +} + static NMCResultCode do_device_wifi_connect (NmCli *nmc, int argc, char **argv) { @@ -3133,6 +3164,7 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) NM80211ApFlags ap_flags; NM80211ApSecurityFlags ap_wpa_flags; NM80211ApSecurityFlags ap_rsn_flags; + NMRemoteConnection *remote_con = NULL; NMConnection *connection = NULL; NMSettingConnection *s_con; NMSettingWireless *s_wifi; @@ -3153,7 +3185,6 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) char *passwd_ask = NULL; const GPtrArray *avail_cons; gboolean name_match = FALSE; - gboolean existing_con = FALSE; int i; /* Set default timeout waiting for operation completion. */ @@ -3402,19 +3433,19 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) /* ap has been checked against bssid1, bssid2 and the ssid * and now avail_con has been checked against ap. */ - connection = NM_CONNECTION (avail_con); - existing_con = TRUE; + remote_con = avail_con; + connection = NM_CONNECTION (remote_con); break; } } - if (name_match && !existing_con) { + if (name_match && !remote_con) { g_string_printf (nmc->return_text, _("Error: Connection '%s' exists but properties don't match."), con_name); nmc->return_value = NMC_RESULT_ERROR_NOT_FOUND; goto finish; } - if (!existing_con) { + if (!remote_con) { /* If there are some connection data from user, create a connection and * fill them into proper settings. */ if (con_name || private || bssid2_arr || hidden) @@ -3533,20 +3564,22 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) info->nmc = nmc; info->device = g_object_ref (device); info->hotspot = FALSE; - info->create = !existing_con; - if (existing_con) { - nm_client_activate_connection_async (nmc->client, - connection, - device, - nm_object_get_path (NM_OBJECT (ap)), - NULL, - add_and_activate_cb, - info); + info->create = !remote_con; + info->specific_object = g_strdup (nm_object_get_path (NM_OBJECT (ap))); + + if (remote_con) { + nm_remote_connection_update2 (remote_con, + nm_connection_to_dbus (connection, NM_CONNECTION_SERIALIZE_ALL), + NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT, + NULL, + NULL, + activate_update2_cb, + info); } else { nm_client_add_and_activate_connection_async (nmc->client, connection, - device, - nm_object_get_path (NM_OBJECT (ap)), + info->device, + info->specific_object, NULL, add_and_activate_cb, info); From eea8bbd9ae9eef1bf27ae02876c6ff99e009b9a6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 3 Aug 2019 11:25:10 +0200 Subject: [PATCH 59/74] libnm: fix leak in NMSettingWireGuard's update_one_secret() --- libnm-core/nm-setting-wireguard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-wireguard.c b/libnm-core/nm-setting-wireguard.c index 07a841f4a7..17c35efc4c 100644 --- a/libnm-core/nm-setting-wireguard.c +++ b/libnm-core/nm-setting-wireguard.c @@ -1922,8 +1922,8 @@ update_one_secret (NMSetting *setting, i_peer = 0; while (g_variant_iter_next (&iter_peers, "@a{sv}", &peer_var)) { _nm_unused gs_unref_variant GVariant *peer_var_unref = peer_var; + nm_auto_unref_wgpeer NMWireGuardPeer *peer = NULL; PeerData *pd; - NMWireGuardPeer *peer; const char *cstr; i_peer++; From 85c26341a280b1234cf73c6537b0bae55fb3adbf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 3 Aug 2019 12:27:57 +0200 Subject: [PATCH 60/74] wireguard: fix use-after free in _peers_remove() --- src/devices/nm-device-wireguard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c index d08f1afd47..e3b8e8b4c4 100644 --- a/src/devices/nm-device-wireguard.c +++ b/src/devices/nm-device-wireguard.c @@ -524,7 +524,7 @@ _peers_remove (NMDeviceWireGuardPrivate *priv, nm_clear_g_cancellable (&peer_data->ep_resolv.cancellable); g_slice_free (PeerData, peer_data); - if (c_list_is_empty (&peer_data->lst_peers)) { + if (c_list_is_empty (&priv->lst_peers_head)) { nm_clear_g_source (&priv->resolve_next_try_id); nm_clear_g_source (&priv->link_config_delayed_id); } From 1634fff1adf71b01e494056669d150fde96018f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 3 Aug 2019 18:33:22 +0200 Subject: [PATCH 61/74] settings: fix registering AgentManager.RegisterWithCapabilities() twice Fixes: 297d4985abcc7b571b8c090ee90622357fc60e16 --- src/settings/nm-agent-manager.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index d361c06a61..2f9827d54a 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -1591,16 +1591,6 @@ static const NMDBusInterfaceInfoExtended interface_info_agent_manager = { ), .handle = impl_agent_manager_register_with_capabilities, ), - NM_DEFINE_DBUS_METHOD_INFO_EXTENDED ( - NM_DEFINE_GDBUS_METHOD_INFO_INIT ( - "RegisterWithCapabilities", - .in_args = NM_DEFINE_GDBUS_ARG_INFOS ( - NM_DEFINE_GDBUS_ARG_INFO ("identifier", "s"), - NM_DEFINE_GDBUS_ARG_INFO ("capabilities", "u"), - ), - ), - .handle = impl_agent_manager_register_with_capabilities, - ), NM_DEFINE_DBUS_METHOD_INFO_EXTENDED ( NM_DEFINE_GDBUS_METHOD_INFO_INIT ( "Unregister", From 956ffb7e9602f7d53e5eb693d1b0155913de45bc Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 5 Aug 2019 09:36:12 +0200 Subject: [PATCH 62/74] settings: fix memory leak Fixes: d35d3c468a304c3e0e78b4b068d105b1d753876c --- src/settings/nm-settings.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 9a17d4774c..42a7bca070 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1455,7 +1455,7 @@ _add_connection_to_first_plugin (NMSettings *self, GError **error) { NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - GError *first_error = NULL; + gs_free_error GError *first_error = NULL; GSList *iter; const char *uuid; @@ -1561,7 +1561,7 @@ _add_connection_to_first_plugin (NMSettings *self, } nm_assert (first_error); - g_propagate_error (error, first_error); + g_propagate_error (error, g_steal_pointer (&first_error)); return FALSE; } From cf6cd06422e2de798c53ce3472255228059b0540 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 08:58:34 +0200 Subject: [PATCH 63/74] cli: add helper function to create and initialize AddAndActivateInfo struct Also use gslice allocator instead of malloc as the size of AddAndActivateInfo is fixed and known beforehand. --- clients/cli/devices.c | 50 +++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 1e199d8af7..33df2c06a1 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1836,17 +1836,37 @@ connected_state_cb (NMDevice *device, NMActiveConnection *active) typedef struct { NmCli *nmc; NMDevice *device; - gboolean hotspot; - gboolean create; char *specific_object; + bool hotspot:1; + bool create:1; } AddAndActivateInfo; +static AddAndActivateInfo * +add_and_activate_info_new (NmCli *nmc, + NMDevice *device, + gboolean hotspot, + gboolean create, + const char *specific_object) +{ + AddAndActivateInfo *info; + + info = g_slice_new (AddAndActivateInfo); + *info = (AddAndActivateInfo) { + .nmc = nmc, + .device = g_object_ref (device), + .hotspot = hotspot, + .create = create, + .specific_object = g_strdup (specific_object), + }; + return info; +} + static void add_and_activate_info_free (AddAndActivateInfo *info) { g_object_unref (info->device); g_free (info->specific_object); - g_free (info); + nm_g_slice_free (info); } static void @@ -1854,7 +1874,7 @@ add_and_activate_cb (GObject *client, GAsyncResult *result, gpointer user_data) { - AddAndActivateInfo *info = (AddAndActivateInfo *) user_data; + AddAndActivateInfo *info = user_data; NmCli *nmc = info->nmc; NMDevice *device = info->device; NMActiveConnection *active; @@ -1925,7 +1945,7 @@ create_connect_connection_for_device (AddAndActivateInfo *info) static void connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) { - AddAndActivateInfo *info = (AddAndActivateInfo *) user_data; + AddAndActivateInfo *info = user_data; NmCli *nmc = info->nmc; NMActiveConnection *active; GError *error = NULL; @@ -2027,10 +2047,7 @@ do_device_connect (NmCli *nmc, int argc, char **argv) nmc); } - info = g_malloc0 (sizeof (AddAndActivateInfo)); - info->nmc = nmc; - info->device = g_object_ref (device); - info->hotspot = FALSE; + info = add_and_activate_info_new (nmc, device, FALSE, FALSE, NULL); nm_client_activate_connection_async (nmc->client, NULL, /* let NM find a connection automatically */ @@ -3131,7 +3148,7 @@ static void activate_update2_cb (GObject *source_object, GAsyncResult *res, gpointer user_data) { NMRemoteConnection *remote_con = NM_REMOTE_CONNECTION (source_object); - AddAndActivateInfo *info = (AddAndActivateInfo *) user_data; + AddAndActivateInfo *info = user_data; NmCli *nmc = info->nmc; gs_unref_variant GVariant *ret = NULL; GError *error = NULL; @@ -3560,12 +3577,7 @@ do_device_wifi_connect (NmCli *nmc, int argc, char **argv) nmc->nowait_flag = (nmc->timeout == 0); nmc->should_wait++; - info = g_malloc0 (sizeof (AddAndActivateInfo)); - info->nmc = nmc; - info->device = g_object_ref (device); - info->hotspot = FALSE; - info->create = !remote_con; - info->specific_object = g_strdup (nm_object_get_path (NM_OBJECT (ap))); + info = add_and_activate_info_new (nmc, device, FALSE, !remote_con, nm_object_get_path (NM_OBJECT (ap))); if (remote_con) { nm_remote_connection_update2 (remote_con, @@ -3932,11 +3944,7 @@ do_device_wifi_hotspot (NmCli *nmc, int argc, char **argv) nmc->nowait_flag = (nmc->timeout == 0); nmc->should_wait++; - info = g_malloc0 (sizeof (AddAndActivateInfo)); - info->nmc = nmc; - info->device = g_object_ref (device); - info->hotspot = TRUE; - info->create = TRUE; + info = add_and_activate_info_new (nmc, device, TRUE, TRUE, NULL); nm_client_add_and_activate_connection_async (nmc->client, connection, From b298f2e6058a5453c73a5a19c273b17b5b4b91b1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Aug 2019 09:10:56 +0200 Subject: [PATCH 64/74] cli: use cleanup macro for freeing AddAndActivateInfo We should prefer the cleanup macors nm_auto*() because they express ownership in code. Also, they allow to return early without additional cleanup code. That way we can refactor if-else blocks. Also, in cases where we intentionally pass on the reference, we use g_steal_pointer(), which literally spells out what happens in code. --- clients/cli/devices.c | 130 +++++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 33df2c06a1..8354fcac75 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1869,16 +1869,19 @@ add_and_activate_info_free (AddAndActivateInfo *info) nm_g_slice_free (info); } +NM_AUTO_DEFINE_FCN0 (AddAndActivateInfo *, _nm_auto_free_add_and_activate_info, add_and_activate_info_free) +#define nm_auto_free_add_and_activate_info nm_auto (_nm_auto_free_add_and_activate_info) + static void add_and_activate_cb (GObject *client, GAsyncResult *result, gpointer user_data) { - AddAndActivateInfo *info = user_data; + nm_auto_free_add_and_activate_info AddAndActivateInfo *info = user_data; NmCli *nmc = info->nmc; NMDevice *device = info->device; - NMActiveConnection *active; - GError *error = NULL; + gs_unref_object NMActiveConnection *active = NULL; + gs_free_error GError *error = NULL; if (info->create) active = nm_client_add_and_activate_connection_finish (NM_CLIENT (client), result, &error); @@ -1886,37 +1889,36 @@ add_and_activate_cb (GObject *client, active = nm_client_activate_connection_finish (NM_CLIENT (client), result, &error); if (error) { - if (info->hotspot) + if (info->hotspot) { g_string_printf (nmc->return_text, _("Error: Failed to setup a Wi-Fi hotspot: %s"), error->message); - else if (info->create) + } else if (info->create) { g_string_printf (nmc->return_text, _("Error: Failed to add/activate new connection: %s"), error->message); - else + } else { g_string_printf (nmc->return_text, _("Error: Failed to activate connection: %s"), error->message); - g_error_free (error); + } nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; quit (); - } else { - if (nmc->nowait_flag) { - g_object_unref (active); - quit (); - } else { - g_object_ref (device); - g_signal_connect (device, "notify::state", G_CALLBACK (device_state_cb), active); - g_signal_connect (active, "notify::state", G_CALLBACK (active_state_cb), device); - - connected_state_cb (device, active); - - g_timeout_add_seconds (nmc->timeout, timeout_cb, nmc); /* Exit if timeout expires */ - - if (nmc->nmc_config.print_output == NMC_PRINT_PRETTY) - progress_id = g_timeout_add (120, progress_cb, device); - } + return; } - add_and_activate_info_free (info); + if (nmc->nowait_flag) { + quit (); + return; + } + + g_signal_connect (device, "notify::state", G_CALLBACK (device_state_cb), active); + g_signal_connect (active, "notify::state", G_CALLBACK (active_state_cb), device); + + connected_state_cb (g_object_ref (device), + g_steal_pointer (&active)); + + g_timeout_add_seconds (nmc->timeout, timeout_cb, nmc); /* Exit if timeout expires */ + + if (nmc->nmc_config.print_output == NMC_PRINT_PRETTY) + progress_id = g_timeout_add (120, progress_cb, device); } static void @@ -1945,9 +1947,9 @@ create_connect_connection_for_device (AddAndActivateInfo *info) static void connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) { - AddAndActivateInfo *info = user_data; + nm_auto_free_add_and_activate_info AddAndActivateInfo *info = user_data; NmCli *nmc = info->nmc; - NMActiveConnection *active; + gs_unref_object NMActiveConnection *active = NULL; GError *error = NULL; const GPtrArray *devices; NMDevice *device; @@ -1958,7 +1960,7 @@ connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) /* If no connection existed for the device, create one and activate it */ if (g_error_matches (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_CONNECTION)) { info->create = TRUE; - create_connect_connection_for_device (info); + create_connect_connection_for_device (g_steal_pointer (&info)); return; } @@ -1967,42 +1969,41 @@ connect_device_cb (GObject *client, GAsyncResult *result, gpointer user_data) g_error_free (error); nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; quit (); - } else { - g_assert (active); - devices = nm_active_connection_get_devices (active); - if (devices->len == 0) { - g_string_printf (nmc->return_text, _("Error: Device activation failed: device was disconnected")); - nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; - g_object_unref (active); - quit (); - add_and_activate_info_free (info); - return; - } - - device = g_ptr_array_index (devices, 0); - - if (nmc->nowait_flag) { - g_object_unref (active); - quit (); - } else { - if (nmc->secret_agent) { - NMRemoteConnection *connection = nm_active_connection_get_connection (active); - - nm_secret_agent_simple_enable (nmc->secret_agent, - nm_connection_get_path (NM_CONNECTION (connection))); - } - - g_object_ref (device); - g_signal_connect (device, "notify::state", G_CALLBACK (device_state_cb), active); - g_signal_connect (active, "notify::state", G_CALLBACK (active_state_cb), device); - - connected_state_cb (device, active); - - /* Start timer not to loop forever if "notify::state" signal is not issued */ - g_timeout_add_seconds (nmc->timeout, timeout_cb, nmc); - } + return; } - add_and_activate_info_free (info); + + nm_assert (NM_IS_ACTIVE_CONNECTION (active)); + + devices = nm_active_connection_get_devices (active); + if (devices->len == 0) { + g_string_printf (nmc->return_text, _("Error: Device activation failed: device was disconnected")); + nmc->return_value = NMC_RESULT_ERROR_CON_ACTIVATION; + quit (); + return; + } + + device = g_ptr_array_index (devices, 0); + + if (nmc->nowait_flag) { + quit (); + return; + } + + if (nmc->secret_agent) { + NMRemoteConnection *connection = nm_active_connection_get_connection (active); + + nm_secret_agent_simple_enable (nmc->secret_agent, + nm_connection_get_path (NM_CONNECTION (connection))); + } + + g_signal_connect (device, "notify::state", G_CALLBACK (device_state_cb), active); + g_signal_connect (active, "notify::state", G_CALLBACK (active_state_cb), device); + + connected_state_cb (g_object_ref (device), + g_steal_pointer (&active)); + + /* Start timer not to loop forever if "notify::state" signal is not issued */ + g_timeout_add_seconds (nmc->timeout, timeout_cb, nmc); } static NMCResultCode @@ -3148,7 +3149,7 @@ static void activate_update2_cb (GObject *source_object, GAsyncResult *res, gpointer user_data) { NMRemoteConnection *remote_con = NM_REMOTE_CONNECTION (source_object); - AddAndActivateInfo *info = user_data; + nm_auto_free_add_and_activate_info AddAndActivateInfo *info = user_data; NmCli *nmc = info->nmc; gs_unref_variant GVariant *ret = NULL; GError *error = NULL; @@ -3160,7 +3161,6 @@ activate_update2_cb (GObject *source_object, GAsyncResult *res, gpointer user_da nmc->return_value = NMC_RESULT_ERROR_UNKNOWN; g_error_free (error); quit (); - add_and_activate_info_free (info); return; } @@ -3170,7 +3170,7 @@ activate_update2_cb (GObject *source_object, GAsyncResult *res, gpointer user_da info->specific_object, NULL, add_and_activate_cb, - info); + g_steal_pointer (&info)); } static NMCResultCode From 66088a09b27450599e9ffb0dbaae01e40ef616bf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 3 Aug 2019 09:04:33 +0200 Subject: [PATCH 65/74] libnm: when stringifying policy routing rule place "not" specifier after "priority" Otherwise, it just looks odd: "not priority 31265 from 0.0.0.0/0 fwmark 0xcb87 table 52103" Better is: "priority 31265 not from 0.0.0.0/0 fwmark 0xcb87 table 52103" The "not" specifier should come after the priority. It makes more sense to read it that way. As far as parsing the string is concerned, the order does not matter. So this change in behavior is no problem. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/228 --- libnm-core/nm-setting-ip-config.c | 9 +++++---- libnm-core/tests/test-setting.c | 4 +++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index 458d93b30d..56b16d414a 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -3110,7 +3110,8 @@ nm_ip_routing_rule_from_string (const char *str, goto next_words_consumed; } if (NM_IN_STRSET (word0, "not")) { - /* we accept multiple "not" specifiers. */ + /* we accept multiple "not" specifiers. "not not" still means + * not. */ val_invert = TRUE; goto next_words_consumed; } @@ -3460,15 +3461,15 @@ nm_ip_routing_rule_to_string (const NMIPRoutingRule *self, str = g_string_sized_new (30); - if (self->invert) - g_string_append (str, "not"); - if (self->priority_has) { g_string_append_printf (nm_gstring_add_space_delimiter (str), "priority %u", (guint) self->priority); } + if (self->invert) + g_string_append (nm_gstring_add_space_delimiter (str), "not"); + _rr_string_append_inet_addr (str, TRUE, ( !self->to_has diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c index aa9de5a03d..3438aea99d 100644 --- a/libnm-core/tests/test-setting.c +++ b/libnm-core/tests/test-setting.c @@ -3168,7 +3168,9 @@ test_routing_rule (gconstpointer test_data) _rr_from_str ("priority 5 from :: iif a\\\\303b table 25"); _rr_from_str ("priority 5 to 0.0.0.0 sport 10 table 6", "priority 5 to 0.0.0.0 sport 10-10 table 6"); - _rr_from_str ("not priority 5 to 0.0.0.0 dport 10-133 table 6", + _rr_from_str ("priority 5 not to 0.0.0.0 dport 10-133 table 6", + "not priority 5 to 0.0.0.0 dport 10-133 table 6", + "not priority 5 not to 0.0.0.0 dport 10-133 table 6", "priority 5 to 0.0.0.0 not dport 10-133 not table 6", "priority 5 to 0.0.0.0 not dport 10-\\ 133 not table 6"); _rr_from_str ("priority 5 to 0.0.0.0 ipproto 10 sport 10 table 6"); From 657b8b31b020938674e220c4e76ce0428285ba96 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 5 Aug 2019 11:01:30 +0200 Subject: [PATCH 66/74] libnm-core: fix ifcfg-rh variable name for DHCPv6 hostname Fixes: 2852b509456d ('ifcfg-rh: add DHCPV6_HOSTNAME and DHCPV6_SEND_HOSTNAME vars') --- libnm-core/nm-setting-ip6-config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-ip6-config.c b/libnm-core/nm-setting-ip6-config.c index bafdb373ce..44379ef727 100644 --- a/libnm-core/nm-setting-ip6-config.c +++ b/libnm-core/nm-setting-ip6-config.c @@ -666,7 +666,7 @@ nm_setting_ip6_config_class_init (NMSettingIP6ConfigClass *klass) /* ---ifcfg-rh--- * property: dhcp-hostname - * variable: DHCP_HOSTNAME + * variable: DHCPV6_HOSTNAME * description: Hostname to send the DHCP server. * ---end--- */ From 9fe2b6135b6fc3dc460c8717ccbcf7f7359f0bd0 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Aug 2019 11:07:34 +0200 Subject: [PATCH 67/74] build: fix meson warning about invalid 'depends' keyword Fix this: libnm/meson.build:215: WARNING: Passed invalid keyword argument "depends". WARNING: This will become a hard error in the future. --- libnm/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm/meson.build b/libnm/meson.build index 3aa182dd4e..2e65d3bebf 100644 --- a/libnm/meson.build +++ b/libnm/meson.build @@ -224,7 +224,7 @@ if enable_introspection extra_args: cflags, header: 'NetworkManager.h', install: true, - depends: libnm_dep, + dependencies: libnm_dep, ) generate_plugin_docs = join_paths(meson.current_source_dir(), 'generate-plugin-docs.pl') From 00bb6cdb4f93f17cbc3f70068d99491eb0fc7726 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Aug 2019 11:18:17 +0200 Subject: [PATCH 68/74] build: fix meson warning about path separator in target Fix the following: WARNING: Target "nm-utils/tests/test-shared-general" has a path separator in its name. This is not supported, it can cause unexpected failures and will become a hard error in the future. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/217 --- shared/meson.build | 24 +++--------------------- shared/nm-utils/tests/meson.build | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 21 deletions(-) create mode 100644 shared/nm-utils/tests/meson.build diff --git a/shared/meson.build b/shared/meson.build index af903d3c8a..d542804eb4 100644 --- a/shared/meson.build +++ b/shared/meson.build @@ -330,24 +330,6 @@ libnm_systemd_shared_no_logging_dep = declare_dependency( ], ) -############################################################################### - -exe = executable( - 'nm-utils/tests/test-shared-general', - [ 'nm-utils/tests/test-shared-general.c' ], - c_args: [ - '-DNETWORKMANAGER_COMPILATION_TEST', - '-DNETWORKMANAGER_COMPILATION=(NM_NETWORKMANAGER_COMPILATION_GLIB|NM_NETWORKMANAGER_COMPILATION_WITH_GLIB_I18N_PROG)', - ], - dependencies: [ - shared_nm_glib_aux_dep, - libnm_systemd_shared_no_logging_dep, - shared_c_siphash_dep, - ], -) - -test( - 'shared/nm-utils/tests/test-shared-general', - test_script, - args: test_args + [exe.full_path()] -) +if enable_tests + subdir('nm-utils/tests') +endif diff --git a/shared/nm-utils/tests/meson.build b/shared/nm-utils/tests/meson.build new file mode 100644 index 0000000000..e0560f296d --- /dev/null +++ b/shared/nm-utils/tests/meson.build @@ -0,0 +1,22 @@ +test_unit = 'test-shared-general' + +exe = executable( + test_unit, + test_unit + '.c', + c_args: [ + '-DNETWORKMANAGER_COMPILATION_TEST', + '-DNETWORKMANAGER_COMPILATION=(NM_NETWORKMANAGER_COMPILATION_GLIB|NM_NETWORKMANAGER_COMPILATION_WITH_GLIB_I18N_PROG)', + ], + dependencies: [ + shared_nm_glib_aux_dep, + libnm_systemd_shared_no_logging_dep, + shared_c_siphash_dep, + ], +) + +test( + 'shared/nm-utils/' + test_unit, + test_script, + args: test_args + [exe.full_path()], + timeout: default_test_timeout, +) From 91b9b08e333bb9bb639803bc133fa2d7e48f3ba0 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Aug 2019 11:37:32 +0200 Subject: [PATCH 69/74] build: fix meson warning about wrong custom target argument src/meson.build:294: WARNING: Custom target input 'NetworkManager' can't be converted to File object(s). This will become a hard error in the future. --- src/meson.build | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/meson.build b/src/meson.build index f3f5ee581a..a9e7601de6 100644 --- a/src/meson.build +++ b/src/meson.build @@ -293,10 +293,9 @@ network_manager_sym = executable( # this uses symbols from nm-full-symbols instead of libNetworkManager.a ver_script = custom_target( symbol_map_name, - input: meson.source_root(), output: symbol_map_name, depends: [ network_manager_sym, core_plugins ], - command: [create_exports_networkmanager, '--called-from-build', '@INPUT@'], + command: [create_exports_networkmanager, '--called-from-build', meson.source_root()], ) ldflags = ['-rdynamic', '-Wl,--version-script,@0@'.format(ver_script.full_path())] From 7c2317a55765f17f6d4213c663728252e0d631af Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 2 Aug 2019 11:59:24 +0200 Subject: [PATCH 70/74] build: fix meson warning about 'install' arg in 'configure_file' WARNING: Project targetting '>= 0.44.0' but tried to use feature introduced in '0.50.0': install arg in configure_file From the documentation: "install (added 0.50.0) When true, this generated file is installed during the install step, and install_dir must be set and not empty. When false, this generated file is not installed regardless of the value of install_dir. When omitted it defaults to true when install_dir is set and not empty, false otherwise." The parameter can be omitted because install_dir is set. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/216 --- data/meson.build | 2 -- dispatcher/meson.build | 1 - 2 files changed, 3 deletions(-) diff --git a/data/meson.build b/data/meson.build index f496bf0135..9025eae452 100644 --- a/data/meson.build +++ b/data/meson.build @@ -11,7 +11,6 @@ server = 'server.conf' configure_file( input: server + '.in', output: server, - install: true, install_dir: join_paths(nm_datadir, 'doc', nm_name, 'examples'), configuration: data_conf, ) @@ -32,7 +31,6 @@ if install_systemd_unit_dir configure_file( input: service + '.in', output: service, - install: true, install_dir: systemd_system_unit_dir, configuration: data_conf, ) diff --git a/dispatcher/meson.build b/dispatcher/meson.build index da9ac7f292..0706ab07a2 100644 --- a/dispatcher/meson.build +++ b/dispatcher/meson.build @@ -13,7 +13,6 @@ service = 'org.freedesktop.nm_dispatcher.service' configure_file( input: service + '.in', output: service, - install: true, install_dir: dbus_sys_dir, configuration: service_conf, ) From 22cd9e754bc7fa233792eb988e5353a1b559b5c4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 6 Aug 2019 08:34:44 +0200 Subject: [PATCH 71/74] modem: fix memory leak Fixes: 9b935fad9b10 ('modem: don't use GAsyncResult pattern for disconnecting modem') --- src/devices/wwan/nm-modem-broadband.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wwan/nm-modem-broadband.c b/src/devices/wwan/nm-modem-broadband.c index 5716500a27..216fedfec6 100644 --- a/src/devices/wwan/nm-modem-broadband.c +++ b/src/devices/wwan/nm-modem-broadband.c @@ -1134,7 +1134,7 @@ simple_disconnect_ready (GObject *source_object, { MMModemSimple *modem_iface = MM_MODEM_SIMPLE (source_object); DisconnectContext *ctx = user_data; - GError *error = NULL; + gs_free_error GError *error = NULL; if (!mm_modem_simple_disconnect_finish (modem_iface, res, &error)) { if ( ctx->warn From f988e85025b63bd7636256842d1e4fea98905c28 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 6 Aug 2019 11:50:00 +0200 Subject: [PATCH 72/74] libnm/doc: add Since tag for %NM_SETTING_IP6_CONFIG_METHOD_DISABLED --- libnm-core/nm-setting-ip6-config.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libnm-core/nm-setting-ip6-config.h b/libnm-core/nm-setting-ip6-config.h index bb590ae456..80449e0c0c 100644 --- a/libnm-core/nm-setting-ip6-config.h +++ b/libnm-core/nm-setting-ip6-config.h @@ -103,6 +103,8 @@ G_BEGIN_DECLS * NM_SETTING_IP6_CONFIG_METHOD_DISABLED: * * IPv6 is disabled for the connection. + * + * Since: 1.20 */ #define NM_SETTING_IP6_CONFIG_METHOD_DISABLED "disabled" From ddb08e3602607a5e137c05c4da25e2b4ab4166dd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 6 Aug 2019 12:10:37 +0200 Subject: [PATCH 73/74] ifupdown: fix assertion during logging %NULL storage in load_eni_ifaces() --- src/settings/plugins/ifupdown/nms-ifupdown-plugin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c index 93f1813cfd..475ecbb6d9 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-plugin.c @@ -321,10 +321,10 @@ load_eni_ifaces (NMSIfupdownPlugin *self) sd_repl = g_hash_table_lookup (eni_ifaces, block->name); if (sd_repl) { - storage = g_steal_pointer (&sd_repl->storage); _LOGD ("parse: replace connection \"%s\" (%s)", block->name, nm_settings_storage_get_uuid (sd_repl->storage)); + storage = g_steal_pointer (&sd_repl->storage); g_hash_table_remove (eni_ifaces, block->name); } From e48089b0393bd502d8ce1f0b44d6c7a6f081a69d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 6 Aug 2019 12:16:22 +0200 Subject: [PATCH 74/74] libnm/doc: add missing "Since: 1.20" comments --- libnm-core/nm-connection.h | 2 +- libnm-core/nm-dbus-interface.h | 4 ++-- libnm-core/nm-setting-wireless.h | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libnm-core/nm-connection.h b/libnm-core/nm-connection.h index 4399ad67da..2c553e0ebd 100644 --- a/libnm-core/nm-connection.h +++ b/libnm-core/nm-connection.h @@ -112,7 +112,7 @@ NMSetting *nm_connection_get_setting_by_name (NMConnection *connection, * @NM_CONNECTION_SERIALIZE_NO_SECRETS: do not include secrets * @NM_CONNECTION_SERIALIZE_ONLY_SECRETS: only serialize secrets * @NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED: if set, only secrets that - * are agent owned will be serialized. + * are agent owned will be serialized. Since: 1.20 * * These flags determine which properties are serialized when calling when * calling nm_connection_to_dbus(). diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h index 1f4b431779..d74ea2ba22 100644 --- a/libnm-core/nm-dbus-interface.h +++ b/libnm-core/nm-dbus-interface.h @@ -292,7 +292,7 @@ typedef enum { /*< flags >*/ * @NM_WIFI_DEVICE_CAP_FREQ_VALID: device reports frequency capabilities * @NM_WIFI_DEVICE_CAP_FREQ_2GHZ: device supports 2.4GHz frequencies * @NM_WIFI_DEVICE_CAP_FREQ_5GHZ: device supports 5GHz frequencies - * @NM_WIFI_DEVICE_CAP_MESH: device supports acting as a mesh point + * @NM_WIFI_DEVICE_CAP_MESH: device supports acting as a mesh point. Since: 1.20. * * 802.11 specific device encryption and authentication capabilities. **/ @@ -385,7 +385,7 @@ typedef enum { /*< underscore_name=nm_802_11_ap_security_flags, flags >*/ * provides connectivity to clients. * @NM_802_11_MODE_AP: the device is an access point/hotspot. Not valid for * access point objects; used only for hotspot mode on the local machine. - * @NM_802_11_MODE_MESH: the device is a 802.11s mesh point. + * @NM_802_11_MODE_MESH: the device is a 802.11s mesh point. Since: 1.20. * * Indicates the 802.11 mode an access point or device is currently in. **/ diff --git a/libnm-core/nm-setting-wireless.h b/libnm-core/nm-setting-wireless.h index dcb11e11c5..d04bad3abc 100644 --- a/libnm-core/nm-setting-wireless.h +++ b/libnm-core/nm-setting-wireless.h @@ -126,6 +126,8 @@ typedef enum { /*< flags >*/ * NM_SETTING_WIRELESS_MODE_MESH: * * Indicates that the connection should create a mesh point. + * + * Since: 1.20 */ #define NM_SETTING_WIRELESS_MODE_MESH "mesh"