Commit Graph

28322 Commits

Author SHA1 Message Date
Thomas Haller
b6a195cf81 tools: remove deprecated PropertiesChanged signal in "test-networkmanager-service.py" 2021-05-14 10:57:35 +02:00
Thomas Haller
48dce1b66c core: drop deprecated PropertiesChanged D-Bus signal (API BREAK)
D-Bus 1.3.1 (2010) introduced the standard "PropertiesChanged" signal
on "org.freedesktop.DBus.Properties". NetworkManager is old, and predates
this API. From that time, it still had it's own PropertiesChanged signal
that are emitted together with the standard ones. NetworkManager
supports the standard PropertiesChanged signal since it switched to
gdbus library in version 1.2.0 (2016).

These own signals are deprecated for a long time already ([1], 2016), and
are hopefully not used by anybody anymore. libnm-glib was using them and
relied on them, but that library is gone. libnm does not use them and neither
does plasma-nm.

Hopefully no users are left that are affected by this API break.

[1] 6fb917178a
2021-05-14 10:57:34 +02:00
Thomas Haller
c8900a437a core: use define DBUS_INTERFACE_PROPERTIES instead of string literal 2021-05-14 10:53:09 +02:00
Thomas Haller
86e99fca6b examples: avoid deprecated PropertiesChanged signal in "create-bond.py" example 2021-05-14 10:53:09 +02:00
Thomas Haller
5f939cc1cb dhcpcd: merge branch 'th/dhcpcd-kill-em-all'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/726

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/856
2021-05-14 10:34:33 +02:00
Thomas Haller
80ced3f1fb dhcpcd: fix killing all processes
With kill(), the PID -1 means to send a signal to all processes.
nm_dhcp_client_get_pid() can return -1, if no PID is set. This
must be handled.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/726

Fixes: a2abd15fe0 ('DHCP: Support dhcpcd-9.x')
2021-05-14 10:34:22 +02:00
Thomas Haller
92bfe09724 dhcp: assert that pid_t is signed for NMDhcpClient
Probably pid_t is always signed, because kill() documents that
negative values have a special meaning (technically, C would
automatically cast negative signed values to an unsigned pid_t type
too).

Anyway, NMDhcpClient at several places uses -1 as special value for "no
pid". At the same time, it checks for valid PIDs with "pid > 1". That
only works if pid_t is signed.

Add a static assertion for that.
2021-05-14 10:34:22 +02:00
Beniamino Galvani
e320beb330 dhcp: nettools: support option 249 (Microsoft Classless Static Route)
From [1]:

  The length and the data format for the Microsoft Classless Static
  Route Option are exactly the same as those specified for the
  Classless Static Route Option in [RFC3442]; the only difference is
  that Option Code 249 should be used instead of or in addition to
  Option Code 121.

Use routes from option 249 when option 121 is not present, as already
done by the dhclient backend.

[1] https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dhcpe/f9c19c79-1c7f-4746-b555-0c0fc523f3f9

https://bugzilla.redhat.com/show_bug.cgi?id=1959461
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/851
2021-05-14 09:26:41 +02:00
Thomas Haller
d7594ae9c9 cli: merge branch 'th/cli-ethtool-ternary'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/849
2021-05-14 09:23:09 +02:00
Thomas Haller
e5b46aa38a cli: use nmc_string_to_ternary() to parse ternary in _set_fcn_ethtool()
nmc_string_to_ternary() is much more flexible, in that it case
insensitive and accepts any unambiguous abbreviation.

Also, nmc_string_to_ternary() accepts "unknown" as alias for "default".

Also, nmc_string_to_ternary() accepts numeric values -1, 0, 1.

By using nmc_string_to_ternary(), we treat ethtool boolean options like
other ternary properties and accept the same keywords.
2021-05-12 22:25:11 +02:00
Thomas Haller
d15eb7dd6e libnmc-base: optionally accept "ignore" in nmc_string_to_ternary() for NMTernary value 2021-05-12 22:25:11 +02:00
Gris Ge
652ddca04c ethtool: Introducing PAUSE support
Introducing ethtool PAUSE support with:

 * ethtool.pause-autoneg on/off
 * ethtool.pause-rx on/off
 * ethtool.pause-tx on/off

Limitations:
 * When `ethtool.pause-autoneg` is set to true, the `ethtool.pause-rx`
   and `ethtool.pause-tx` will be ignored. We don't have warning for
   this yet.

Unit test case included.

Signed-off-by: Gris Ge <fge@redhat.com>

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/829
2021-05-12 18:04:46 +02:00
Thomas Haller
d0e8a7e2ac ifcfg-rh: merge branch 'th/ifcfg-write-wired-setting'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/848
2021-05-12 13:45:20 +02:00
Thomas Haller
0956354bc5 ifcfg-rh: for ethernet profiles write TYPE before other wired settings 2021-05-12 13:43:37 +02:00
Thomas Haller
6f3f25cead ifcfg-rh: write all [ethernet] settings for write_wired_for_virtual()
It's not the task of the writer to mangle/normalize profiles. If a profile
for a virtual device can have an [ethernet] setting, then unsuitable values
like s390 options must be either rejected by nm_connection_verify() or normalized
by nm_connection_normalize(). In no way it's right that the writer simple
pretends they are not set.
2021-05-12 13:43:36 +02:00
Thomas Haller
166c458411 ifcfg-rh: refactor common parts of write_wired_setting()/write_wired_for_virtual() 2021-05-12 13:43:36 +02:00
Thomas Haller
a21714b821 ifcfg-rh: cleanup write_wired_for_virtual() to return-early 2021-05-12 13:43:36 +02:00
Thomas Haller
46eb75d746 ifcfg-rh/trivial: move code 2021-05-12 13:43:36 +02:00
Thomas Haller
18ca3803b5 contrib/rpm: merge branch 'th/contrib-rpm-fixes'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/850
2021-05-12 13:36:50 +02:00
Thomas Haller
af40a14556 contrib/rpm: %ghost "/etc/NetworkManager/VPN"
"/etc/NetworkManager/VPN" was historically the place for .name files for
VPN plugins. In the meantime, those should be under "/usr/lib/NetworkManager/VPN".

Still, NetworkManager honors (and possibly watches) the directory in
/etc. Mark the directory as %ghost.

The exact effect of this is not clear to me. It seems however right to
do, and works for my testing.
2021-05-12 13:36:19 +02:00
Thomas Haller
40c78fe699 contrib/rpm: obsolete "config-routing-rules" package with fixed version
Since commit a447942fc0 ('contrib/rpm: rename package
"NetworkManager-config-routing-rules" to
"NetworkManager-dispatcher-routing-rules"'), the config-routing-rules
subpackage is gone.

This way to specify the version number with a variable parameter, causes
repeated messages in rpmdiff:

    INFO  NetworkManager-dispatcher-routing-rules  changed from Obsoletes: NetworkManager-config-routing-rules < 1:1.32.0-0.2.el8 to Obsoletes: NetworkManager-config-routing-rules < 1:1.32.0-0.3.el8 on noarch

Avoid this by hard coding the obsoleted version.
2021-05-12 13:35:37 +02:00
Thomas Haller
9e3b7b2e99 contrib/rpm: let NetworkManager-libnm package conflict with exact NetworkManager-glib version
This "Conflicts" is since commit b85b8ed6fa ('contrib/rpm: let
NetworkManager-libnm and NetworkManager-glib of differing version
conflict'). This was probably fine back then, but NetworkManager-glib is
long gone.

Also, not hard coding the version number leads to rpmdiff messages like:

  NEEDS INSPECTION  NetworkManager-libnm  changed from Conflicts: NetworkManager-glib < 1:1.32.0-0.2.el8 to Conflicts: NetworkManager-glib < 1:1.32.0-0.3.el8 on all architectures

As NetworkManager-glib is long gone, hard code the version with which
we conflict.
2021-05-12 13:35:28 +02:00
Thomas Haller
d784fdb9d9 glib-aux: use nm_str_buf_append_c() in nm_str_buf_append_c_hex() 2021-05-12 10:06:21 +02:00
Thomas Haller
77fb782060 glib-aux: avoid potential undefined behavior for nm_str_buf_append_printf()
The string buffer may be empty and _priv_str still %NULL. Doing
pointer arithmetic with a %NULL pointer is undefined behavior.
Avoid that.

It's probably not an issue, because it results in computing &(((char *) NULL)[0],
and then g_vsnprintf() would not even inspect the pointer (so it doesn't
matter whether the computed pointer is bogus). But still, there is
undefined behavior involved.
2021-05-12 09:38:15 +02:00
Thomas Haller
4bc9c59c07 wifi: refactor parsing secrutiy flags of supplicant in security_from_vardict()
I think if-else is easier to read. It also makes it clear that the
options are really exclusive, and for each option only one flag is set.
That was not easy to see previously.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/844
2021-05-11 14:03:37 +02:00
Thomas Haller
17f45689bf all: merge branch 'th/coverity-fix-some-leaks'
https://bugzilla.redhat.com/show_bug.cgi?id=1938672

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/843
2021-05-11 14:00:17 +02:00
Thomas Haller
e5f37477c0 cli: fix leak in print_wifi_connection()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772): [#def297] [important]
  NetworkManager-1.31.3/src/nmcli/devices.c:4610: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
  NetworkManager-1.31.3/src/nmcli/devices.c:4610: var_assign: Assigning: "ssid" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid_bytes, NULL), g_bytes_get_size(ssid_bytes))".
  NetworkManager-1.31.3/src/nmcli/devices.c:4612: noescape: Resource "ssid" is not freed or pointed-to in "g_print".
  NetworkManager-1.31.3/src/nmcli/devices.c:4642: noescape: Resource "ssid" is not freed or pointed-to in "string_append_mecard".
  NetworkManager-1.31.3/src/nmcli/devices.c:4654: leaked_storage: Variable "ssid" going out of scope leaks the storage it points to.
  # 4652|
  # 4653|       g_print("\n");
  # 4654|-> }
  # 4655|
  # 4656|   static gboolean

Fixes: 7061341a41 ('cli: add "nmcli d wifi show"')
2021-05-11 13:56:50 +02:00
Thomas Haller
61029d4064 libnmt-newt: use cleanup macro in nmt_newt_button_build_component() 2021-05-11 13:56:50 +02:00
Thomas Haller
853f411567 libnmt-newt: fix leak in nmt_newt_button_build_component()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772): [#def274] [important]
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:118: alloc_fn: Storage is returned from allocation function "g_strdup_printf".
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:118: var_assign: Assigning: "label" = storage returned from "g_strdup_printf(" <%s>", priv->label)".
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:119: noescape: Resource "label" is not freed or pointed-to in "nmt_newt_locale_from_utf8".
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:125: leaked_storage: Variable "label" going out of scope leaks the storage it points to.
  #  123|       }
  #  124|
  #  125|->     return co;
  #  126|   }
  #  127|

Fixes: 3bda3fb60c ('nmtui: initial import of nmtui')
2021-05-11 13:56:49 +02:00
Thomas Haller
2c628e4762 libnmc-base:fix leak in NMSecretAgentSimple's request_secrets_from_ui()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772): [#def271] [important]
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: var_assign: Assigning: "ssid_utf8" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid, NULL), g_bytes_get_size(ssid))".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:877: noescape: Resource "ssid_utf8" is not freed or pointed-to in "g_strdup_printf".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:882: leaked_storage: Variable "ssid_utf8" going out of scope leaks the storage it points to.
  #  880|
  #  881|           if (!add_wireless_secrets(request, secrets))
  #  882|->             goto out_fail;
  #  883|       } else if (nm_connection_is_type(request->connection, NM_SETTING_WIRED_SETTING_NAME)) {
  #  884|           title = _("Wired 802.1X authentication");

  Error: RESOURCE_LEAK (CWE-772): [#def272] [important]
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: var_assign: Assigning: "ssid_utf8" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid, NULL), g_bytes_get_size(ssid))".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:877: noescape: Resource "ssid_utf8" is not freed or pointed-to in "g_strdup_printf".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:883: leaked_storage: Variable "ssid_utf8" going out of scope leaks the storage it points to.
  #  881|           if (!add_wireless_secrets(request, secrets))
  #  882|               goto out_fail;
  #  883|->     } else if (nm_connection_is_type(request->connection, NM_SETTING_WIRED_SETTING_NAME)) {
  #  884|           title = _("Wired 802.1X authentication");
  #  885|           msg   = g_strdup_printf(_("Secrets are required to access the wired network %s"),

Fixes: 3fbabde4c3 ('libnm-core: replace GByteArray with pointer + length in some APIs')
2021-05-11 13:56:49 +02:00
Thomas Haller
d527d3874c glib-aux: workaround coverty warning about comparing nm_json_int_t with int64
Error: CONSTANT_EXPRESSION_RESULT (CWE-569): [#def240]
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-json-aux.h:260: result_independent_of_operands: "v < -9223372036854775808LL /* (gint64)(-9223372036854775807L - 1L) */" is always false regardless of the values of its operands. This occurs as the logical first operand of "||".
  #  258|
  #  259|       v = vt->nm_json_integer_value(elem);
  #  260|->     if (v < G_MININT64 || v > G_MAXINT64)
  #  261|           return -ERANGE;
  #  262|

  Error: CONSTANT_EXPRESSION_RESULT (CWE-569): [#def241]
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-json-aux.h:279: result_independent_of_operands: "v > 18446744073709551615UL" is always false regardless of the values of its operands. This occurs as the logical second operand of "||".
  #  277|
  #  278|       v = vt->nm_json_integer_value(elem);
  #  279|->     if (v < 0 || v > G_MAXUINT64)
  #  280|           return -ERANGE;
  #  281|
2021-05-11 13:56:48 +02:00
Thomas Haller
463db1c7a6 tui: use internal _nm_utils_ip4_prefix_to_netmask() helper
nm_utils_ip4_prefix_to_netmask() is public API of libnm.
As we also want to have this function at a few places where
we don't have libnm, we have an internal variant
_nm_utils_ip4_prefix_to_netmask().

Use the internal variant consistently and everywhere.
2021-05-11 13:56:48 +02:00
Thomas Haller
b487cf30a9 glib-aux/tests: add test for _nm_utils_ip4_prefix_to_netmask() 2021-05-11 13:56:47 +02:00
Thomas Haller
64985beef8 dhcp/systemd: avoid using g_free() with buffers allocated with malloc() in lease_to_ip4_config()()
Coverity says:

  Error: ALLOC_FREE_MISMATCH (CWE-762):
  NetworkManager-1.31.3/src/core/dhcp/nm-dhcp-systemd.c:234: alloc: Allocation of memory which must be freed using "free".
  NetworkManager-1.31.3/src/core/dhcp/nm-dhcp-systemd.c:447: free: Calling "_nm_auto_g_free" frees "routes" using "g_free" but it should have been freed using "free".
  #  445|       }
  #  446|       NM_SET_OUT(out_options, g_steal_pointer(&options));
  #  447|->     return g_steal_pointer(&ip4_config);
  #  448|   }
  #  449|

Fixes: acc0d79224 ('systemd: merge branch 'systemd' into master')
2021-05-11 13:56:47 +02:00
Thomas Haller
44abe6d661 systemd/tests: avoid using g_free() with buffers allocated with malloc() in _test_unbase64mem_mem()
Coverity says:

  Error: ALLOC_FREE_MISMATCH (CWE-762):
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:261: alloc: Allocation of memory which must be freed using "free".
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:274: free: Calling "_nm_auto_g_free" frees "exp2_arr" using "g_free" but it should have been freed using "free".
  #  272|           g_assert_cmpmem(expected_arr, expected_len, exp3_arr, exp3_len);
  #  273|       }
  #  274|-> }
  #  275|
  #  276|   #define _test_unbase64mem(base64, expected_str) \

  Error: ALLOC_FREE_MISMATCH (CWE-762):
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:270: alloc: Allocation of memory which must be freed using "free".
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:274: free: Calling "_nm_auto_g_free" frees "exp3_arr" using "g_free" but it should have been freed using "free".
  #  272|           g_assert_cmpmem(expected_arr, expected_len, exp3_arr, exp3_len);
  #  273|       }
  #  274|-> }
  #  275|
  #  276|   #define _test_unbase64mem(base64, expected_str) \

Fixes: 0298d54078 ('systemd: expose unbase64mem() as nm_sd_utils_unbase64mem()')
2021-05-11 13:56:47 +02:00
Thomas Haller
936b60e00f wifi: avoid large shift for calculating netmask in ip4_config_to_iwd_config()
Found by Coverity:

  Error: BAD_SHIFT (CWE-682): [#def53]
  NetworkManager-1.31.3/src/core/devices/wifi/nm-wifi-utils.c:1590: zero_return: Function call "nm_ip_address_get_prefix(addr)" returns 0.
  NetworkManager-1.31.3/src/core/devices/wifi/nm-wifi-utils.c:1590: assignment: Assigning: "prefix" = "nm_ip_address_get_prefix(addr)". The value of "prefix" is now 0.
  NetworkManager-1.31.3/src/core/devices/wifi/nm-wifi-utils.c:1591: large_shift: In expression "0xffffffffU << 32U - prefix", left shifting by more than 31 bits has undefined behavior.  The shift amount, "32U - prefix", is 32.
  # 1589|           NMIPAddress *addr    = nm_setting_ip_config_get_address(s_ip, 0);
  # 1590|           guint        prefix  = nm_ip_address_get_prefix(addr);
  # 1591|->         in_addr_t    netmask = htonl(0xffffffffu << (32 - prefix));
  # 1592|           char         buf[INET_ADDRSTRLEN];
  # 1593|

Fixes: 9d22ae7981 ('wifi: Add utilities for writing IWD connection profiles')
2021-05-11 13:56:46 +02:00
Thomas Haller
820ab364fd tests: avoid Coverity warning UNINIT in nmtst_keyfile_get_num_keys()
A false positive:

  Error: UNINIT (CWE-457):
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-test-utils.h:2816: var_decl: Declaring variable "l" without initializer.
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-test-utils.h:2828: uninit_use: Using uninitialized value "l".
  # 2826|       nmtst_assert_success(keys, error);
  # 2827|
  # 2828|->     g_assert_cmpint(NM_PTRARRAY_LEN(keys), ==, l);
  # 2829|
  # 2830|       return l;
2021-05-11 13:56:46 +02:00
Thomas Haller
90f5d9114f libnm: use nm_auto_unref_tc_action cleanup macro in nm_utils_tc_action_from_str() 2021-05-11 13:56:45 +02:00
Thomas Haller
3cd56e92d4 libnm: fix leak in nm_utils_tc_tfilter_from_str()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772):
  NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2772: alloc_fn: Storage is returned from allocation function "nm_utils_tc_action_from_str".
  NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2772: var_assign: Assigning: "action" = storage returned from "nm_utils_tc_action_from_str(extra_opts, error)".
  NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2785: leaked_storage: Variable "action" going out of scope leaks the storage it points to.
  # 2783|       tfilter = nm_tc_tfilter_new(kind, parent, error);
  # 2784|       if (!tfilter)
  # 2785|->         return NULL;
  # 2786|
  # 2787|       nm_tc_tfilter_set_handle(tfilter, handle);

Fixes: de41c45e61 ('libnm-core: add functionality for dealing with tc-style traffic filter specifiers')
2021-05-11 13:56:45 +02:00
Thomas Haller
272119d925 libnm: add nm_auto_unref_tc_action cleanup macro for NMTCAction 2021-05-11 13:56:45 +02:00
Thomas Haller
02dbba49d6 libnm: fix leak in nm_vpn_service_plugin_read_vpn_details()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772):
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:814: alloc_fn: Storage is returned from allocation function "g_string_new".
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:814: var_assign: Assigning: "key" = storage returned from "g_string_new(line->str + strlen("DATA_KEY="))".
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:815: var_assign: Assigning: "str" = "key".
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:855: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:855: leaked_storage: Variable "key" going out of scope leaks the storage it points to.
  #  853|           NM_SET_OUT(out_secrets, g_steal_pointer(&secrets));
  #  854|       }
  #  855|->     return success;
  #  856|   }
  #  857|

Fixes: 3dfb72b926 ('service-plugin: allow continuations in the auth-dialog protocol')
2021-05-11 13:56:45 +02:00
Thomas Haller
ceaa1c369f core: fix leak in _config_data_get_main_auth_polkit()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772):
  NetworkManager-1.31.3/src/core/nm-config-data.c:450: alloc_fn: Storage is returned from allocation function "nm_config_data_get_value".
  NetworkManager-1.31.3/src/core/nm-config-data.c:450: var_assign: Assigning: "str" = storage returned from "nm_config_data_get_value(self, "main", "auth-polkit", (enum [unnamed type of NMConfigGetValueFlags])6)".
  NetworkManager-1.31.3/src/core/nm-config-data.c:454: noescape: Resource "str" is not freed or pointed-to in "nm_auth_polkit_mode_from_string".
  NetworkManager-1.31.3/src/core/nm-config-data.c:465: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
  #  463|           NM_SET_OUT(out_invalid_config, FALSE);
  #  464|
  #  465|->     return auth_polkit_mode;
  #  466|   }
  #  467|

Fixes: 6d7446e52f ('core: add main.auth-polkit option "root-only"')
2021-05-11 13:56:44 +02:00
Thomas Haller
dd3aa1224a ifcfg-rh: use NMStrBuf in svUnescape()
This is a popular, low-level function. Let's use NMStrBuf.

Also, Coverity wrongly things that there is a leak here. This change
should also avoid that:

    Error: RESOURCE_LEAK (CWE-772):
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/shvar.c:411: alloc_arg: "_gstr_init" allocates memory that is stored into "str".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/shvar.c:423: noescape: Resource "str" is not freed or pointed-to in "g_string_append_len".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/shvar.c:619: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
    #  617|           nm_assert(!str);
    #  618|           *to_free = NULL;
    #  619|->         return "";
    #  620|       }
    #  621|

Profile:

We run test-ifcfg-rh which calls svUnescape() under realistic circumstances.
However, the test does too many other things that svUnescape() would be
measurable. So use the following patch, to run the tested code more frequently:

    diff --git a/src/core/settings/plugins/ifcfg-rh/shvar.c b/src/core/settings/plugins/ifcfg-rh/shvar.c
    index c6099dd1731c..18a907113ea9 100644
    --- a/src/core/settings/plugins/ifcfg-rh/shvar.c
    +++ b/src/core/settings/plugins/ifcfg-rh/shvar.c
    @@ -645,6 +645,24 @@ out_error:
         return NULL;
     }

    +#define svUnescape(value, to_free)                   \
    +    ({                                               \
    +        const char *_value = (value);                \
    +        const char *_result;                         \
    +        int         _i;                              \
    +                                                     \
    +        for (_i = 0; TRUE; _i++) {                   \
    +            gs_free char *_to_free;                  \
    +                                                     \
    +            _result = svUnescape(_value, &_to_free); \
    +            if (_i < 1000)                           \
    +                continue;                            \
    +            *(to_free) = g_steal_pointer(&_to_free); \
    +            break;                                   \
    +        }                                            \
    +        _result;                                     \
    +    })
    +
     /*****************************************************************************/

     shvarFile *

Build:

    CFLAGS='-O2' ./autogen.sh --with-more-asserts=0
    make -j 10 src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh && \
        src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh &&
        perf stat -r 50 -B src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh

Before:

 Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (20 runs):

            590.56 msec task-clock:u              #    0.972 CPUs utilized            ( +-  0.48% )
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
             1,091      page-faults:u             #    0.002 M/sec                    ( +-  0.12% )
     2,022,618,453      cycles:u                  #    3.425 GHz                      ( +-  0.33% )
     4,165,011,633      instructions:u            #    2.06  insn per cycle           ( +-  0.01% )
     1,168,673,648      branches:u                # 1978.910 M/sec                    ( +-  0.01% )
         8,279,364      branch-misses:u           #    0.71% of all branches          ( +-  0.14% )

           0.60739 +- 0.00292 seconds time elapsed  ( +-  0.48% )

After:

 Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (50 runs):

            580.19 msec task-clock:u              #    0.972 CPUs utilized            ( +-  0.33% )
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
             1,092      page-faults:u             #    0.002 M/sec                    ( +-  0.08% )
     1,956,368,933      cycles:u                  #    3.372 GHz                      ( +-  0.22% )
     4,106,984,148      instructions:u            #    2.10  insn per cycle           ( +-  0.01% )
     1,087,931,864      branches:u                # 1875.143 M/sec                    ( +-  0.01% )
         7,731,041      branch-misses:u           #    0.71% of all branches          ( +-  0.15% )

           0.59680 +- 0.00193 seconds time elapsed  ( +-  0.32% )

The run time varies greatly. But it can be seen that the new code is consistently
faster.
2021-05-11 13:56:23 +02:00
Thomas Haller
1556732ef0 glib-aux: add nm_str_buf_append_unichar() helper 2021-05-11 13:53:52 +02:00
Thomas Haller
7065d75b91 ifcfg-rh: avoid cloning vlans array in write_bridge_vlans() 2021-05-11 13:53:52 +02:00
Thomas Haller
72e8336fdc libnm: add internal accessors for _nm_setting_bridge{,_port}_get_vlans()
For internal code, when we control what we are doing, make it possible
to directly access the internal GPtrArray.
2021-05-11 13:53:52 +02:00
Thomas Haller
f5685e5bc9 ifcfg-rh: add comment about unreachable code in write_bridge_vlans() 2021-05-11 13:53:52 +02:00
Thomas Haller
5b36f215f4 ifcfg-rh: fix code that looks like a leak in write_bridge_vlans()
"string" is leaked in the error case. But in practice, this cannot
happen because nm_bridge_vlan_to_str() cannot fail.

While at it, replace GString by NMStrBuf.

Thanks Coverity:

    Error: RESOURCE_LEAK (CWE-772):
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1565: alloc_fn: Storage is returned from allocation function "g_string_new".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1565: var_assign: Assigning: "string" = storage returned from "g_string_new("")".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1572: leaked_storage: Variable "string" going out of scope leaks the storage it points to.
    # 1570|           vlan_str = nm_bridge_vlan_to_str(vlan, error);
    # 1571|           if (!vlan_str)
    # 1572|->             return FALSE;
    # 1573|           if (string->len > 0)
    # 1574|               g_string_append(string, ",");
2021-05-11 13:53:52 +02:00
Thomas Haller
8e5374512e CONTRIBUTING: add section about unit tests 2021-05-11 12:24:38 +02:00
Fernando Fernandez Mancera
eee4332e8f ifcfg: fix ACCEPT_ALL_MAC_ADDRESSES for virtual interfaces
The rh-ifcfg plugin was missing the writting code for
ACCEPT_ALL_MAC_ADDRESSES property in virtual interfaces.

https://bugzilla.redhat.com/show_bug.cgi?id=1942331

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>

Fixes: d946aa0c50 ('wired-setting: add support to accept-all-mac-addresses')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/846
2021-05-10 22:54:50 +02:00