From 54a64edefc4222b8ef20d837c885618ea5f6e0d7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 22:48:34 +0200 Subject: [PATCH 01/16] libnm: don't compare invalid mac addresses as equal in nm_utils_hwaddr_matches() By passing as length of the MAC addresses -1 for both arguments, one could get through to compare empty strings, NULL, and addresses longer than the maximum. Such addresses are not valid, and they should never compare equal (not even to themselves). This is a change in behavior of public API, but it never made sense to claim two addresses are equal, when they are not even valid addresses. Also, avoid undefined behavior with "NULL, -1, NULL, -1" arguments, where we would call memcmp() with zero length and NULL arguments. UBSan flags that too. --- libnm-core/nm-utils.c | 15 ++++++++++++--- libnm-core/tests/test-general.c | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index f9bccea930..59798c1d9b 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -4269,7 +4269,8 @@ nm_utils_hwaddr_matches (gconstpointer hwaddr1, hwaddr1 = buf1; hwaddr1_len = l; } else { - g_return_val_if_fail ((hwaddr2_len == -1 && hwaddr2) || (hwaddr2_len > 0 && hwaddr2_len <= NM_UTILS_HWADDR_LEN_MAX), FALSE); + g_return_val_if_fail ( hwaddr2_len == -1 + || (hwaddr2_len > 0 && hwaddr2_len <= NM_UTILS_HWADDR_LEN_MAX), FALSE); return FALSE; } } else { @@ -4301,9 +4302,17 @@ nm_utils_hwaddr_matches (gconstpointer hwaddr1, } } + if (G_UNLIKELY ( hwaddr1_len <= 0 + || hwaddr1_len > NM_UTILS_HWADDR_LEN_MAX)) { + /* Only valid addresses can compare equal. In particular, + * addresses that are too long or of zero bytes, never + * compare equal. */ + return FALSE; + } + if (hwaddr1_len == INFINIBAND_ALEN) { - hwaddr1 = (guint8 *)hwaddr1 + INFINIBAND_ALEN - 8; - hwaddr2 = (guint8 *)hwaddr2 + INFINIBAND_ALEN - 8; + hwaddr1 = &((guint8 *) hwaddr1)[INFINIBAND_ALEN - 8]; + hwaddr2 = &((guint8 *) hwaddr2)[INFINIBAND_ALEN - 8]; hwaddr1_len = 8; } diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 52f05dc306..a6fb700715 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -4278,7 +4278,7 @@ test_hwaddr_equal (void) g_assert (nm_utils_hwaddr_matches (null_binary, sizeof (null_binary), null_binary, sizeof (null_binary))); g_assert (nm_utils_hwaddr_matches (null_binary, sizeof (null_binary), NULL, ETH_ALEN)); - g_assert (nm_utils_hwaddr_matches (NULL, -1, NULL, -1)); + g_assert (!nm_utils_hwaddr_matches (NULL, -1, NULL, -1)); g_assert (!nm_utils_hwaddr_matches (NULL, -1, string, -1)); g_assert (!nm_utils_hwaddr_matches (string, -1, NULL, -1)); g_assert (!nm_utils_hwaddr_matches (NULL, -1, null_string, -1)); From 18b903943d3c7fb81eded28dfe92c96bb661cd86 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 22:12:09 +0200 Subject: [PATCH 02/16] cli: fix memcpy() with %NULL pointers in nmc_get_devices_sorted() UBSan correctly flags this: clients/cli/devices.c:966:2: runtime error: null pointer passed as argument 2, which is declared to never be null --- clients/cli/devices.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 2d79d2ae9a..4998ff032e 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -963,7 +963,8 @@ nmc_get_devices_sorted (NMClient *client) devs = nm_client_get_devices (client); sorted = g_new (NMDevice *, devs->len + 1); - memcpy (sorted, devs->pdata, devs->len * sizeof (NMDevice *)); + if (devs->len > 0) + memcpy (sorted, devs->pdata, devs->len * sizeof (NMDevice *)); sorted[devs->len] = NULL; qsort (sorted, devs->len, sizeof (NMDevice *), compare_devices); From 589d51ca9dcb58ad758d4b94f08bbf5d63cbf46b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 11:51:54 +0200 Subject: [PATCH 03/16] cli: fix leak in show_device_lldp_list() --- clients/cli/devices.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 4998ff032e..88ff00c03b 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -4600,10 +4600,10 @@ show_device_lldp_list (NMDevice *device, NmCli *nmc, const char *fields_str, int set_val_strc (arr, 13, str); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_CHASSIS_ID_TYPE, &value)) - set_val_strc (arr, 14, g_strdup_printf ("%u", value)); + set_val_str (arr, 14, g_strdup_printf ("%u", value)); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_PORT_ID_TYPE, &value)) - set_val_strc (arr, 15, g_strdup_printf ("%u", value)); + set_val_str (arr, 15, g_strdup_printf ("%u", value)); g_ptr_array_add (out.output_data, arr); } From a1e12c01df2ffaf388b0445179c9be70af0e2431 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 12:03:16 +0200 Subject: [PATCH 04/16] cli: fix leak in show_device_lldp_list() for nmc_parse_lldp_capabilities() --- clients/cli/devices.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 88ff00c03b..b85c1205cd 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -4578,8 +4578,11 @@ show_device_lldp_list (NMDevice *device, NmCli *nmc, const char *fields_str, int if (nm_lldp_neighbor_get_attr_string_value (neighbor, NM_LLDP_ATTR_SYSTEM_DESCRIPTION, &str)) set_val_strc (arr, 6, str); - if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_SYSTEM_CAPABILITIES, &value)) - set_val_str (arr, 7, g_strdup_printf ("%u (%s)", value, nmc_parse_lldp_capabilities (value))); + if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_SYSTEM_CAPABILITIES, &value)) { + gs_free char *tmp = NULL; + + set_val_str (arr, 7, g_strdup_printf ("%u (%s)", value, (tmp = nmc_parse_lldp_capabilities (value)))); + } if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_IEEE_802_1_PVID, &value)) set_val_str (arr, 8, g_strdup_printf ("%u", value)); From 446a145db5d4262bd393acf27899f4970b6f3859 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 11:53:37 +0200 Subject: [PATCH 05/16] cli: use nm_strdup_int() in "clients/cli/devices.c" --- clients/cli/devices.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index b85c1205cd..6a9bb77b7c 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -1192,7 +1192,7 @@ fill_output_access_point (gpointer data, gpointer user_data) channel_str = g_strdup_printf ("%u", nm_utils_wifi_freq_to_channel (freq)); freq_str = g_strdup_printf (_("%u MHz"), freq); bitrate_str = g_strdup_printf (_("%u Mbit/s"), bitrate/1000); - strength_str = g_strdup_printf ("%u", strength); + strength_str = nm_strdup_int (strength); wpa_flags_str = ap_wpa_rsn_flags_to_string (wpa_flags, NM_META_ACCESSOR_GET_TYPE_PRETTY); rsn_flags_str = ap_wpa_rsn_flags_to_string (rsn_flags, NM_META_ACCESSOR_GET_TYPE_PRETTY); sig_bars = nmc_wifi_strength_bars (strength); @@ -4585,16 +4585,16 @@ show_device_lldp_list (NMDevice *device, NmCli *nmc, const char *fields_str, int } if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_IEEE_802_1_PVID, &value)) - set_val_str (arr, 8, g_strdup_printf ("%u", value)); + set_val_str (arr, 8, nm_strdup_int (value)); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_IEEE_802_1_PPVID, &value)) - set_val_str (arr, 9, g_strdup_printf ("%u", value)); + set_val_str (arr, 9, nm_strdup_int (value)); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_IEEE_802_1_PPVID_FLAGS, &value)) - set_val_str (arr, 10, g_strdup_printf ("%u", value)); + set_val_str (arr, 10, nm_strdup_int (value)); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_IEEE_802_1_VID, &value)) - set_val_str (arr, 11, g_strdup_printf ("%u", value)); + set_val_str (arr, 11, nm_strdup_int (value)); if (nm_lldp_neighbor_get_attr_string_value (neighbor, NM_LLDP_ATTR_IEEE_802_1_VLAN_NAME, &str)) set_val_strc (arr, 12, str); @@ -4603,10 +4603,10 @@ show_device_lldp_list (NMDevice *device, NmCli *nmc, const char *fields_str, int set_val_strc (arr, 13, str); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_CHASSIS_ID_TYPE, &value)) - set_val_str (arr, 14, g_strdup_printf ("%u", value)); + set_val_str (arr, 14, nm_strdup_int (value)); if (nm_lldp_neighbor_get_attr_uint_value (neighbor, NM_LLDP_ATTR_PORT_ID_TYPE, &value)) - set_val_str (arr, 15, g_strdup_printf ("%u", value)); + set_val_str (arr, 15, nm_strdup_int (value)); g_ptr_array_add (out.output_data, arr); } From 14bf28f109b64adaed96219e0e82a6ff8d63b96d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 22:34:05 +0200 Subject: [PATCH 06/16] tests: fix uint32 integer constants for TC_H_MAKE() UBSan marks these: libnm-core/tests/test-setting.c:2146:2: runtime error: left shift of 65521 by 16 places cannot be represented in type 'int' #0 0x561739bed935 in test_tc_config_qdisc libnm-core/tests/test-setting.c:2146 --- libnm-core/tests/test-setting.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c index 1e82e245f9..32eac0236b 100644 --- a/libnm-core/tests/test-setting.c +++ b/libnm-core/tests/test-setting.c @@ -2142,8 +2142,8 @@ test_tc_config_qdisc (void) nmtst_assert_success (qdisc1, error); g_assert_cmpstr (nm_tc_qdisc_get_kind (qdisc1), ==, "pfifo_fast"); - g_assert (nm_tc_qdisc_get_handle (qdisc1) == TC_H_MAKE (0x1234 << 16, 0x0000)); - g_assert (nm_tc_qdisc_get_parent (qdisc1) == TC_H_MAKE (0xfff1 << 16, 0x0001)); + g_assert (nm_tc_qdisc_get_handle (qdisc1) == TC_H_MAKE (0x1234u << 16, 0x0000u)); + g_assert (nm_tc_qdisc_get_parent (qdisc1) == TC_H_MAKE (0xfff1u << 16, 0x0001u)); str = nm_utils_tc_qdisc_to_str (qdisc1, &error); nmtst_assert_success (str, error); @@ -2224,12 +2224,12 @@ test_tc_config_tfilter (void) GError *error = NULL; tfilter1 = nm_tc_tfilter_new ("matchall", - TC_H_MAKE (0x1234 << 16, 0x0000), + TC_H_MAKE (0x1234u << 16, 0x0000u), &error); nmtst_assert_success (tfilter1, error); tfilter2 = nm_tc_tfilter_new ("matchall", - TC_H_MAKE (0x1234 << 16, 0x0000), + TC_H_MAKE (0x1234u << 16, 0x0000u), &error); nmtst_assert_success (tfilter2, error); @@ -2283,10 +2283,10 @@ test_tc_config_setting_valid (void) nmtst_assert_success (qdisc1, error); qdisc2 = nm_tc_qdisc_new ("pfifo_fast", - TC_H_MAKE (0xfff1 << 16, 0x0001), + TC_H_MAKE (0xfff1u << 16, 0x0001u), &error); nmtst_assert_success (qdisc2, error); - nm_tc_qdisc_set_handle (qdisc2, TC_H_MAKE (0x1234 << 16, 0x0000)); + nm_tc_qdisc_set_handle (qdisc2, TC_H_MAKE (0x1234u << 16, 0x0000u)); g_assert (nm_setting_tc_config_get_num_qdiscs (s_tc) == 0); g_assert (nm_setting_tc_config_add_qdisc (s_tc, qdisc1) == TRUE); @@ -2389,16 +2389,16 @@ test_tc_config_dbus (void) qdisc1 = nm_tc_qdisc_new ("fq_codel", TC_H_ROOT, &error); nmtst_assert_success (qdisc1, error); - nm_tc_qdisc_set_handle (qdisc1, TC_H_MAKE (0x1234 << 16, 0x0000)); + nm_tc_qdisc_set_handle (qdisc1, TC_H_MAKE (0x1234u << 16, 0x0000u)); nm_setting_tc_config_add_qdisc (NM_SETTING_TC_CONFIG (s_tc), qdisc1); qdisc2 = nm_tc_qdisc_new ("ingress", TC_H_INGRESS, &error); nmtst_assert_success (qdisc2, error); - nm_tc_qdisc_set_handle (qdisc2, TC_H_MAKE (TC_H_INGRESS, 0)); + nm_tc_qdisc_set_handle (qdisc2, TC_H_MAKE (TC_H_INGRESS, 0u)); nm_setting_tc_config_add_qdisc (NM_SETTING_TC_CONFIG (s_tc), qdisc2); tfilter1 = nm_tc_tfilter_new ("matchall", - TC_H_MAKE (0x1234 << 16, 0x0000), + TC_H_MAKE (0x1234u << 16, 0x0000u), &error); nmtst_assert_success (tfilter1, error); action = nm_tc_action_new ("drop", &error); @@ -2409,7 +2409,7 @@ test_tc_config_dbus (void) nm_tc_tfilter_unref (tfilter1); tfilter2 = nm_tc_tfilter_new ("matchall", - TC_H_MAKE (TC_H_INGRESS, 0), + TC_H_MAKE (TC_H_INGRESS, 0u), &error); nmtst_assert_success (tfilter2, error); action = nm_tc_action_new ("simple", &error); From 1473f00d74e0cc5f96de157354df5a74b792a59d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 09:41:37 +0200 Subject: [PATCH 07/16] gitlab-ci: accept failure in REQUIRED_PACKAGES script for non-existing qt-devel On CentOS 8, many devel packages are not available. Even after # dnf config-manager --set-enabled PowerTools certain devel packages are missing. Some of these (libndp-devel, mobile-broadband-provider-info-devel, teamd-devel) we build in copr ([1]), but libpsl-devel and qt-devel are still missing. Only install them optionally and allow failure for them not being present. [1] https://copr.fedorainfracloud.org/coprs/nmstate/nm-build-deps/repo/epel-8/nmstate-nm-build-deps-epel-8.repo --- contrib/fedora/REQUIRED_PACKAGES | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/fedora/REQUIRED_PACKAGES b/contrib/fedora/REQUIRED_PACKAGES index eb2404a170..96d70fff1f 100755 --- a/contrib/fedora/REQUIRED_PACKAGES +++ b/contrib/fedora/REQUIRED_PACKAGES @@ -59,7 +59,6 @@ install \ jansson-devel \ libcurl-devel \ libndp-devel \ - libpsl-devel \ libselinux-devel \ libtool \ libuuid-devel \ @@ -73,7 +72,6 @@ install \ ppp-devel \ python3-dbus \ python3-gobject \ - qt-devel \ readline-devel \ rpm-build \ systemd-devel \ @@ -86,7 +84,9 @@ install \ # some packages don't exist in certain distributions. Install them one-by-one, and ignore errors. install_ignore_missing \ - python-gobject-base \ dbus-python \ + libpsl-devel \ pygobject3-base \ + python-gobject-base \ + qt-devel \ #end From b846f9aba3e854b728595b8043338eb4be324abd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 10:15:45 +0200 Subject: [PATCH 08/16] gitlab-ci: optionally install libasan,libubsan via REQUIRED_PACKAGES script --- contrib/fedora/REQUIRED_PACKAGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/fedora/REQUIRED_PACKAGES b/contrib/fedora/REQUIRED_PACKAGES index 96d70fff1f..0850bc345f 100755 --- a/contrib/fedora/REQUIRED_PACKAGES +++ b/contrib/fedora/REQUIRED_PACKAGES @@ -85,7 +85,9 @@ install \ # some packages don't exist in certain distributions. Install them one-by-one, and ignore errors. install_ignore_missing \ dbus-python \ + libasan \ libpsl-devel \ + libubsan \ pygobject3-base \ python-gobject-base \ qt-devel \ From 0a030da6c25100e6e272b8db5b2fd9601f5fc0e6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 22:56:33 +0200 Subject: [PATCH 09/16] gitlab-ci: add more CentOS images for tests --- .gitlab-ci.yml | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 37dbae173e..b52dd29b8e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -12,10 +12,15 @@ stages: .fedora_install: &fedora_install before_script: - # enable EPEL on CentOS - - date '+%Y%m%d-%H%M%S'; ! grep -q '^NAME=.*\(CentOS\)' /etc/os-release || yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<7\>' /etc/os-release ) || yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<7\>' /etc/os-release ) || yum install -y glibc-common + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<7\>' /etc/os-release ) || localedef -c -i pl_PL -f UTF-8 pl_PL.UTF-8 && locale -a + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<7\>' /etc/os-release ) || yum install -y python36-dbus python36-gobject-base - - date '+%Y%m%d-%H%M%S'; ! grep -q '^NAME=.*\(CentOS\)' /etc/os-release || (yum install -y glibc-common && localedef -c -i pl_PL -f UTF-8 pl_PL.UTF-8 && locale -a) + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<8\>' /etc/os-release ) || dnf install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-8.noarch.rpm + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<8\>' /etc/os-release ) || dnf install -y 'dnf-command(config-manager)' + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<8\>' /etc/os-release ) || dnf config-manager --set-enabled PowerTools + - date '+%Y%m%d-%H%M%S'; ! ( grep -q '^NAME=.*\(CentOS\)' /etc/os-release && grep -q '^VERSION_ID=.*\<8\>' /etc/os-release ) || curl https://copr.fedorainfracloud.org/coprs/nmstate/nm-build-deps/repo/epel-8/nmstate-nm-build-deps-epel-8.repo > /etc/yum.repos.d/nmstate-nm-build-deps-epel-8.repo - date '+%Y%m%d-%H%M%S'; NM_NO_EXTRA=1 NM_INSTALL="yum install -y" ./contrib/fedora/REQUIRED_PACKAGES - date '+%Y%m%d-%H%M%S'; yum install -y glibc-langpack-pl ccache clang which @@ -24,8 +29,6 @@ stages: # to generate proper documentation. - date '+%Y%m%d-%H%M%S'; yum reinstall -y --setopt='tsflags=' glib2-doc - - date '+%Y%m%d-%H%M%S'; ! grep -q '^NAME=.*\(CentOS\)' /etc/os-release || yum install -y python36-dbus python36-gobject-base - - date '+%Y%m%d-%H%M%S'; ! which dnf || dnf install -y python3-dnf-plugins-core - date '+%Y%m%d-%H%M%S'; ! which dnf || dnf debuginfo-install -y glib2 - date '+%Y%m%d-%H%M%S'; which dnf || debuginfo-install -y glib2 @@ -138,6 +141,24 @@ t_centos:7.6.1810: <<: *do_build when: manual +t_centos:7.7.1908: + <<: *fedora_install + image: centos:7.7.1908 + <<: *do_build + when: manual + +t_centos:7.8.2003: + <<: *fedora_install + image: centos:7.8.2003 + <<: *do_build + when: manual + +t_centos:8.1.1911: + <<: *fedora_install + image: centos:8.1.1911 + <<: *do_build + when: manual + t_ubuntu:16.04: <<: *debian_install image: ubuntu:16.04 From 42d45299f97d90a1a8a0309e54a72df72527980a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 22:26:05 +0200 Subject: [PATCH 10/16] tests/sanitizer: make ASAN/LSAN/UBSAN options configurable in "tools/run-nm-test.sh" Also add a suppressions file for LSAN. --- Makefile.am | 1 + lsan.suppressions | 0 tools/run-nm-test.sh | 13 +++++++++++++ 3 files changed, 14 insertions(+) create mode 100644 lsan.suppressions diff --git a/Makefile.am b/Makefile.am index 35564aa5c5..1a2456afc6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5097,6 +5097,7 @@ EXTRA_DIST += \ src/ppp/nm-ppp-plugin.ver \ Makefile.glib \ autogen.sh \ + lsan.suppressions \ valgrind.suppressions \ meson.build \ meson_options.txt \ diff --git a/lsan.suppressions b/lsan.suppressions new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools/run-nm-test.sh b/tools/run-nm-test.sh index 16d9d8a7ab..02fced0164 100755 --- a/tools/run-nm-test.sh +++ b/tools/run-nm-test.sh @@ -228,6 +228,19 @@ if [[ -n "$BUILDDIR" ]]; then fi fi +export ASAN_OPTIONS="$NM_TEST_ASAN_OPTIONS" +export LSAN_OPTIONS="$NM_TEST_LSAN_OPTIONS" +export UBSAN_OPTIONS="$NM_TEST_UBSAN_OPTIONS" +if [ -z "${NM_TEST_ASAN_OPTIONS+x}" ]; then + ASAN_OPTIONS="fast_unwind_on_malloc=false detect_leaks=1" +fi +if [ -z "${NM_TEST_LSAN_OPTIONS+x}" ]; then + LSAN_OPTIONS="suppressions=$SCRIPT_PATH/../lsan.suppressions" +fi +if [ -z "${NM_TEST_UBSAN_OPTIONS+x}" ]; then + UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1" +fi + if ! _is_true "$NMTST_USE_VALGRIND" 0; then export NM_TEST_UNDER_VALGRIND=0 exec "${NMTST_DBUS_RUN_SESSION[@]}" \ From 8113bc22d4b90519867052b8b28293793470ace2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 11:03:03 +0200 Subject: [PATCH 11/16] tests/sanitizer: suppress leak in openssl Suppress a leak report from openssl: Direct leak of 192 byte(s) in 1 object(s) allocated from: #0 0x7f6fe9c6b677 in __interceptor_malloc (/lib64/libasan.so.6+0xb0677) #1 0x7f6fe4d4046d in CRYPTO_zalloc crypto/mem.c:230 #2 0x7f6fe4d0a91f in ENGINE_new crypto/engine/eng_lib.c:34 #3 0x7f6fe4d0c40d in ENGINE_rdrand crypto/engine/eng_rdrand.c:70 #4 0x7f6fe4d0c40d in engine_load_rdrand_int crypto/engine/eng_rdrand.c:85 #5 0x7f6fe4d370ec in ossl_init_engine_rdrand crypto/init.c:353 #6 0x7f6fe4d370ec in ossl_init_engine_rdrand_ossl_ crypto/init.c:347 #7 0x7f6fe995aace in __pthread_once_slow (/lib64/libpthread.so.0+0x11ace) #8 0x7f6fe4da68fc in CRYPTO_THREAD_run_once crypto/threads_pthread.c:118 #9 0x7f6fe4d378ec in OPENSSL_init_crypto crypto/init.c:723 #10 0x7f6fe4d378ec in OPENSSL_init_crypto crypto/init.c:620 #11 0x7f6fe5292280 (/usr/lib64/pkcs11/libsofthsm2.so+0x78280) #12 0x7f6fe5292364 (/usr/lib64/pkcs11/libsofthsm2.so+0x78364) #13 0x7f6fe526f151 in SoftHSM::C_Initialize(void*) /usr/src/debug/softhsm-2.5.0-4.fc32.3.x86_64/src/lib/SoftHSM.cpp:485 #14 0x7f6fe523cc97 in C_Initialize (/usr/lib64/pkcs11/libsofthsm2.so+0x22c97) #15 0x7f6fe4ecb233 in initialize_module_inlock_reentrant ../p11-kit/modules.c:738 #16 0x7f6fe4ecb382 in managed_C_Initialize ../p11-kit/modules.c:1584 #17 0x7f6fe4ecdbdf in p11_kit_modules_initialize ../p11-kit/modules.c:2157 #18 0x7f6fe4ecdbdf in p11_kit_modules_initialize ../p11-kit/modules.c:2145 #19 0x7f6fe4ed1a96 in proxy_create ../p11-kit/proxy.c:330 #20 0x7f6fe4ed1a96 in proxy_C_Initialize ../p11-kit/proxy.c:398 #21 0x7f6fe9a343b1 in secmod_ModuleInit /usr/src/debug/nss-3.51.0-1.fc32.x86_64/nss/lib/pk11wrap/pk11load.c:244 #22 0x7f6fe9a34adb in secmod_LoadPKCS11Module /usr/src/debug/nss-3.51.0-1.fc32.x86_64/nss/lib/pk11wrap/pk11load.c:501 #23 0x7f6fe9a419ec in SECMOD_LoadModule /usr/src/debug/nss-3.51.0-1.fc32.x86_64/nss/lib/pk11wrap/pk11pars.c:1840 #24 0x7f6fe9a41b27 in SECMOD_LoadModule /usr/src/debug/nss-3.51.0-1.fc32.x86_64/nss/lib/pk11wrap/pk11pars.c:1876 #25 0x7f6fe9a0dd00 in nss_Init /usr/src/debug/nss-3.51.0-1.fc32.x86_64/nss/lib/nss/nssinit.c:712 #26 0x7f6fe9a0e3ab in NSS_NoDB_Init /usr/src/debug/nss-3.51.0-1.fc32.x86_64/nss/lib/nss/nssinit.c:950 #27 0x55c942e2f1b2 in _nm_crypto_init libnm-core/nm-crypto-nss.c:61 #28 0x55c942d6f2da in nm_crypto_load_and_verify_certificate libnm-core/nm-crypto.c:721 #29 0x55c942c99681 in _cert_impl_set libnm-core/nm-setting-8021x.c:497 #30 0x55c942c9d83b in nm_setting_802_1x_set_ca_cert libnm-core/nm-setting-8021x.c:1033 #31 0x55c942c63513 in _test_8021x_cert_from_files libnm-core/tests/test-keyfile.c:382 #32 0x55c942c6425a in test_8021x_cert libnm-core/tests/test-keyfile.c:436 #33 0x7f6fe965429d in test_case_run ../glib/gtestutils.c:2633 #34 0x7f6fe965429d in g_test_run_suite_internal ../glib/gtestutils.c:2721 --- lsan.suppressions | 1 + 1 file changed, 1 insertion(+) diff --git a/lsan.suppressions b/lsan.suppressions index e69de29bb2..205e2741f6 100644 --- a/lsan.suppressions +++ b/lsan.suppressions @@ -0,0 +1 @@ +leak:OPENSSL_init_crypto From c6234e114bc3dc5779451cf29634337879f0c6be Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 22:17:50 +0200 Subject: [PATCH 12/16] clients/tests: preserve caller's ASAN/LSAN/UBSAN environment variables for client tests --- clients/tests/test-client.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/clients/tests/test-client.py b/clients/tests/test-client.py index daa69842c8..934cec60d3 100755 --- a/clients/tests/test-client.py +++ b/clients/tests/test-client.py @@ -63,6 +63,10 @@ ENV_NM_TEST_REGENERATE = 'NM_TEST_REGENERATE' # numbers enabled. ENV_NM_TEST_WITH_LINENO = 'NM_TEST_WITH_LINENO' +ENV_NM_TEST_ASAN_OPTIONS = 'NM_TEST_ASAN_OPTIONS' +ENV_NM_TEST_LSAN_OPTIONS = 'NM_TEST_LSAN_OPTIONS' +ENV_NM_TEST_UBSAN_OPTIONS = 'NM_TEST_UBSAN_OPTIONS' + # ############################################################################### @@ -406,6 +410,17 @@ class Configuration: v = (os.environ.get(ENV_NM_TEST_REGENERATE, '0') == '1') elif name == ENV_NM_TEST_WITH_LINENO: v = (os.environ.get(ENV_NM_TEST_WITH_LINENO, '0') == '1') + elif name in [ ENV_NM_TEST_ASAN_OPTIONS, ENV_NM_TEST_LSAN_OPTIONS, ENV_NM_TEST_UBSAN_OPTIONS ]: + v = os.environ.get(name, None) + if v is None: + if name == ENV_NM_TEST_ASAN_OPTIONS: + v = 'detect_leaks=0' + elif name == ENV_NM_TEST_LSAN_OPTIONS: + v = '' + elif name == ENV_NM_TEST_UBSAN_OPTIONS: + v = '' + else: + assert(False) else: raise Exception() self._values[name] = v @@ -763,7 +778,9 @@ class TestNmcli(NmTestBase): env['LIBNM_USE_SESSION_BUS'] = '1' env['LIBNM_USE_NO_UDEV'] = '1' env['TERM'] = 'linux' - env['ASAN_OPTIONS'] = 'detect_leaks=0' + env['ASAN_OPTIONS'] = conf.get(ENV_NM_TEST_ASAN_OPTIONS) + env['LSAN_OPTIONS'] = conf.get(ENV_NM_TEST_LSAN_OPTIONS) + env['LBSAN_OPTIONS'] = conf.get(ENV_NM_TEST_UBSAN_OPTIONS) env['XDG_CONFIG_HOME'] = PathConfiguration.srcdir() env['NM_TEST_CALLING_NUM'] = str(calling_num) if fatal_warnings is _DEFAULT_ARG or fatal_warnings: From 5198bce5ee4acf1ba1c7bd641fee089ebb4dc057 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 11:30:33 +0200 Subject: [PATCH 13/16] clients/tests: set "UBSAN_OPTIONS" to abort tests and set "ASAN_OPTIONS=detect_leaks=1" --- clients/tests/test-client.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clients/tests/test-client.py b/clients/tests/test-client.py index 934cec60d3..8bbbdbfc47 100755 --- a/clients/tests/test-client.py +++ b/clients/tests/test-client.py @@ -414,11 +414,12 @@ class Configuration: v = os.environ.get(name, None) if v is None: if name == ENV_NM_TEST_ASAN_OPTIONS: - v = 'detect_leaks=0' + v = 'detect_leaks=1' + #v += ' fast_unwind_on_malloc=false' elif name == ENV_NM_TEST_LSAN_OPTIONS: v = '' elif name == ENV_NM_TEST_UBSAN_OPTIONS: - v = '' + v = 'print_stacktrace=1:halt_on_error=1' else: assert(False) else: From 1acb351848e3718d168726b0b5de3fb598b31d43 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 17:39:54 +0200 Subject: [PATCH 14/16] build/autotools: reject invalid sanitizer options in configure.ac --- configure.ac | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/configure.ac b/configure.ac index 2d5e7241c8..960f957afd 100644 --- a/configure.ac +++ b/configure.ac @@ -1137,6 +1137,10 @@ if test "$with_address_sanitizer" = yes -o "$with_address_sanitizer" = "exec"; t else sanitizers="${sanitizers}address(executables-only) " fi +elif test "$with_address_sanitizer" = "" -o "$with_address_sanitizer" = no; then + with_address_sanitizer=no +else + AC_MSG_ERROR(["Invalid asan option --with-address-sanitizer=$with_address_sanitizer"]) fi AC_ARG_ENABLE(undefined-sanitizer, @@ -1152,6 +1156,10 @@ if test "${enable_undefined_sanitizer}" = "yes"; then sanitizer_exec_ldflags="$sanitizer_exec_ldflags -Wc,-fsanitize=undefined" sanitizer_lib_ldflags="$sanitizer_lib_ldflags -Wc,-fsanitize=undefined" sanitizers="${sanitizers}undefined-behavior " +elif test "$enable_undefined_sanitizer" = "" -o "$enable_undefined_sanitizer" = no; then + enable_undefined_sanitizer=no +else + AC_MSG_ERROR(["Invalid asan option --enable-undefined-sanitizer=$enable_undefined_sanitizer"]) fi if test -n "$sanitizers"; then From 9119e5b618d871d218a296b2cac8fc999e7a433c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 May 2020 21:59:50 +0200 Subject: [PATCH 15/16] build/autotools: fix linking nm-online,nm-dispatcher,nm-bt-test with sanitizer flags --- Makefile.am | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index 1a2456afc6..d8cd32fab9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2391,7 +2391,8 @@ src_NetworkManager_LDADD = \ src_NetworkManager_LDFLAGS = \ -rdynamic \ -Wl,--version-script="src/NetworkManager.ver" \ - $(SANITIZER_EXEC_LDFLAGS) + $(SANITIZER_EXEC_LDFLAGS) \ + $(NULL) $(src_NetworkManager_OBJECTS): $(libnm_core_lib_h_pub_mkenums) @@ -3592,6 +3593,11 @@ check_programs_norun += \ src_devices_bluetooth_tests_nm_bt_test_CPPFLAGS = \ $(src_cppflags_test) \ $(NULL) + +src_devices_bluetooth_tests_nm_bt_test_LDFLAGS = \ + $(SANITIZER_EXEC_LDFLAGS) \ + $(NULL) + src_devices_bluetooth_tests_nm_bt_test_LDADD = \ src/devices/bluetooth/libnm-bluetooth-utils.la \ src/libNetworkManager.la \ @@ -4164,7 +4170,9 @@ dispatcher_nm_dispatcher_SOURCES = \ dispatcher_nm_dispatcher_CPPFLAGS = $(dispatcher_cppflags) dispatcher_nm_dispatcher_LDFLAGS = \ - -Wl,--version-script="$(srcdir)/linker-script-binary.ver" + -Wl,--version-script="$(srcdir)/linker-script-binary.ver" \ + $(SANITIZER_EXEC_LDFLAGS) \ + $(NULL) dispatcher_nm_dispatcher_LDADD = \ dispatcher/libnm-dispatcher-core.la \ @@ -4282,7 +4290,9 @@ clients_nm_online_CPPFLAGS = \ $(NULL) clients_nm_online_LDFLAGS = \ - -Wl,--version-script="$(srcdir)/linker-script-binary.ver" + -Wl,--version-script="$(srcdir)/linker-script-binary.ver" \ + $(SANITIZER_EXEC_LDFLAGS) \ + $(NULL) clients_nm_online_LDADD = \ libnm/libnm.la \ From 350681e7f1bdb89d1d4cf17bcef86cd18b7f1caf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 14 May 2020 10:18:12 +0200 Subject: [PATCH 16/16] contrib/rpm: enable undefined-sanitizer libubsan on RHEL-8 --- contrib/fedora/rpm/NetworkManager.spec | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 5717d9d208..8f9a473cb5 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -238,7 +238,7 @@ BuildRequires: polkit-devel BuildRequires: jansson-devel %if %{with sanitizer} BuildRequires: libasan -%if 0%{?fedora} +%if 0%{?fedora} || 0%{?rhel} >= 8 BuildRequires: libubsan %endif %endif @@ -641,8 +641,10 @@ intltoolize --automake --copy --force %endif %if %{with sanitizer} --with-address-sanitizer=exec \ -%if 0%{?fedora} +%if 0%{?fedora} || 0%{?rhel} >= 8 --enable-undefined-sanitizer \ +%else + --disable-undefined-sanitizer \ %endif %else --with-address-sanitizer=no \