From fd5945b4084d72e3b77346a3ee363a9d3633cab7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 12 May 2022 09:42:16 +0200 Subject: [PATCH 1/2] libnm: fix crash validating infiniband profiles for interface-name A virtual infiniband profile (with p-key>=0) can also contain a "connection.interface-name". But it is required to match the f"{parent}.{p-key}" format. However, such a profile can also set "mac_address" instead of "parent". In that case, the validation code was crashing. nmcli connection add type infiniband \ infiniband.p-key 6 \ infiniband.mac-address 52:54:00:86:f4:eb:aa:aa:aa:aa:52:54:00:86:f4:eb:aa:aa:aa:aa \ connection.interface-name aaaa The crash was introduced by commit 99d898cf1fa2 ('libnm: rework caching of virtual-iface-name for infiniband setting'). Previously, it would not have crashed, because we just called g_strdup_printf("%s.%04x", priv->parent, priv->p_key) with a NULL string. It would still not have validated the connection and passing NULL as string to printf is wrong. But in practice, it would have worked mostly fine for users. Fixes: 99d898cf1fa2 ('libnm: rework caching of virtual-iface-name for infiniband setting') --- src/libnm-core-impl/nm-setting-infiniband.c | 26 ++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-infiniband.c b/src/libnm-core-impl/nm-setting-infiniband.c index eb6c9536e..6bd195617 100644 --- a/src/libnm-core-impl/nm-setting-infiniband.c +++ b/src/libnm-core-impl/nm-setting-infiniband.c @@ -254,17 +254,27 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) virtual_iface_name = nm_setting_infiniband_get_virtual_interface_name(NM_SETTING_INFINIBAND(setting)); - if (!nm_streq(interface_name, virtual_iface_name)) { + if (!nm_streq0(interface_name, virtual_iface_name)) { /* We don't support renaming software infiniband devices. Later we might, but * for now just reject such connections. **/ - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("interface name of software infiniband device must be '%s' or unset " - "(instead it is '%s')"), - virtual_iface_name, - interface_name); + if (virtual_iface_name) { + g_set_error( + error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("interface name of software infiniband device must be '%s' or unset " + "(instead it is '%s')"), + virtual_iface_name, + interface_name); + } else { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("interface name of software infiniband device with MAC address " + "must be unset (instead it is '%s')"), + interface_name); + } g_prefix_error(error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, From 7012b9001a3ebf3231b41163e60c7b4c2e894c4e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 12 May 2022 10:02:18 +0200 Subject: [PATCH 2/2] libnm: reject infiniband.p-key set to 0, 0x8000 Kernel does not allow this ([1], [2]). Usually tightening the verification is a break of API. But in this case, no user had a working configuration that is breaking. At worst, they had a broken profile that no longer loads. We also filter those from _infiniband_add_add_or_delete(), since [3]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/infiniband/ulp/ipoib/ipoib_main.c?id=f443e374ae131c168a065ea1748feac6b2e76613#n2394 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/infiniband/ulp/ipoib/ipoib_vlan.c?id=f443e374ae131c168a065ea1748feac6b2e76613#n116 [3] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/eab817d34a38227a79b10e9c52d450bb8c7fa907 --- src/libnm-core-impl/nm-setting-infiniband.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libnm-core-impl/nm-setting-infiniband.c b/src/libnm-core-impl/nm-setting-infiniband.c index 6bd195617..787b838b7 100644 --- a/src/libnm-core-impl/nm-setting-infiniband.c +++ b/src/libnm-core-impl/nm-setting-infiniband.c @@ -241,6 +241,14 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) g_prefix_error(error, "%s: ", NM_SETTING_INFINIBAND_PARENT); return FALSE; } + if (NM_IN_SET(priv->p_key, 0, 0x8000)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("the values 0 and 0x8000 are not allowed")); + g_prefix_error(error, "%s: ", NM_SETTING_INFINIBAND_P_KEY); + return FALSE; + } } if (connection)