From 6e10bdf37ad6b318de9a37416a3a80656d407006 Mon Sep 17 00:00:00 2001 From: Lukas Nykryn Date: Fri, 8 Dec 2023 12:33:06 +0100 Subject: [PATCH 5004/5004] udev: allow/denylist for reading sysfs attributes when composing a NIC name Users can currently pick specific versions of NIC naming, but that does not guarantee that NIC names won't change after the kernel adds a new sysfs attribute. This patch allows for an allow/deny list of sysfs attributes that could be used when composing the name. These lists can be supplied as an hwdb entry in the form of /etc/udev/hwdb.d/50-net-naming-allowlist.hwdb net:naming:drvirtio_net ID_NET_NAME_ALLOW=0 ID_NET_NAME_ALLOW_ACPI_INDEX=1 ID_NET_NAME_ALLOW_ADDR_ASSIGN_TYPE=1 ID_NET_NAME_ALLOW_ADDRESS=1 ID_NET_NAME_ALLOW_ARI_ENABLED=1 ID_NET_NAME_ALLOW_DEV_PORT=1 ID_NET_NAME_ALLOW_FUNCTION_ID=1 ID_NET_NAME_ALLOW_IFLINK=1 ID_NET_NAME_ALLOW_INDEX=1 ID_NET_NAME_ALLOW_LABEL=1 ID_NET_NAME_ALLOW_PHYS_PORT_NAME=1 ID_NET_NAME_ALLOW_TYPE=1 (cherry picked from commit 3b2e7dc5a285edbbb1bf6aed2d88b889d801613f) [fbui: adjust context] [fbui: fixes bsc#1234015] --- man/systemd.net-naming-scheme.xml | 69 ++++++++++++++++++++++++++ rules.d/75-net-description.rules | 2 + src/shared/netif-naming-scheme.c | 81 +++++++++++++++++++++++++++++++ src/shared/netif-naming-scheme.h | 7 +++ src/udev/udev-builtin-net_id.c | 38 +++++++-------- 5 files changed, 178 insertions(+), 19 deletions(-) diff --git a/man/systemd.net-naming-scheme.xml b/man/systemd.net-naming-scheme.xml index 3d997535d4..a8e23b1862 100644 --- a/man/systemd.net-naming-scheme.xml +++ b/man/systemd.net-naming-scheme.xml @@ -485,6 +485,45 @@ particular version of systemd). + + Limiting the use of specific sysfs attributes + + When creating names for network cards, some naming schemes use data from sysfs populated + by the kernel. This means that although a specific naming scheme in udev is picked, + the network card's name can still change when a new kernel version adds a new sysfs attribute. + For example if kernel starts setting the phys_port_name, udev will append the + "nphys_port_name" suffix to the device name. + + + + ID_NET_NAME_ALLOW=BOOL + + This evironment value sets a fallback policy for reading a sysfs attribute. + If set to 0 udev will not read any sysfs attribute by default, unless it is + explicitly allowlisted, see below. If set to 1 udev can use any sysfs attribute + unless it is explicitly forbidden. The default value is 1. + + + + + + ID_NET_NAME_ALLOW_sysfsattr=BOOL + + This evironment value explicitly states if udev shall use the specified + sysfsattr, when composing the device name. + + + + + + With these options, users can set an allowlist or denylist for sysfs attributes. To create + an allowlist, the user needs to set ID_NET_NAME_ALLOW=0 for the device and then list + the allowed attributes with the + ID_NET_NAME_ALLOW_sysfsattr=1 + options. In case of a denylist, the user needs to provide the list of denied attributes with + the ID_NET_NAME_ALLOW_sysfsattr=0 options. + + Examples @@ -571,6 +610,36 @@ ID_NET_NAME_PATH=enp0s29u1u2 ID_NET_NAME_MAC=enx026d3c00000a ID_NET_NAME_PATH=encf5f0 + + + Set an allowlist for reading sysfs attributes for network card naming + + /etc/udev/hwdb.d/50-net-naming-allowlist.hwdb +net:naming:drvirtio_net:* + ID_NET_NAME_ALLOW=0 + ID_NET_NAME_ALLOW_ACPI_INDEX=1 + ID_NET_NAME_ALLOW_ADDR_ASSIGN_TYPE=1 + ID_NET_NAME_ALLOW_ADDRESS=1 + ID_NET_NAME_ALLOW_ARI_ENABLED=1 + ID_NET_NAME_ALLOW_DEV_PORT=1 + ID_NET_NAME_ALLOW_FUNCTION_ID=1 + ID_NET_NAME_ALLOW_IFLINK=1 + ID_NET_NAME_ALLOW_INDEX=1 + ID_NET_NAME_ALLOW_LABEL=1 + ID_NET_NAME_ALLOW_PHYS_PORT_NAME=1 + ID_NET_NAME_ALLOW_TYPE=1 + + + + Set a denylist so that specified sysfs attribute are ignored + + /etc/udev/hwdb.d/50-net-naming-denylist.hwdb +net:naming:drvirtio_net:* + ID_NET_NAME_ALLOW=1 + ID_NET_NAME_ALLOW_DEV_PORT=0 + ID_NET_NAME_ALLOW_PHYS_PORT_NAME=0 + + diff --git a/rules.d/75-net-description.rules b/rules.d/75-net-description.rules index 7e62f8b26b..5ba70a6545 100644 --- a/rules.d/75-net-description.rules +++ b/rules.d/75-net-description.rules @@ -3,6 +3,8 @@ ACTION=="remove", GOTO="net_end" SUBSYSTEM!="net", GOTO="net_end" +IMPORT{builtin}="hwdb 'net:naming:dr$env{ID_NET_DRIVER}:'" + IMPORT{builtin}="net_id" SUBSYSTEMS=="usb", IMPORT{builtin}="usb_id", IMPORT{builtin}="hwdb --subsystem=usb" diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index b6a97527d8..77c0bcd57a 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -1,6 +1,9 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "sd-device.h" + #include "alloc-util.h" +#include "device-private.h" #include "netif-naming-scheme.h" #include "proc-cmdline.h" #include "string-util.h" @@ -106,3 +109,81 @@ static const char* const alternative_names_policy_table[_NAMEPOLICY_MAX] = { }; DEFINE_STRING_TABLE_LOOKUP(alternative_names_policy, NamePolicy); + +static int naming_sysattr_allowed_by_default(sd_device *dev) { + int r; + + assert(dev); + + r = device_get_property_bool(dev, "ID_NET_NAME_ALLOW"); + if (r == -ENOENT) + return true; + + return r; +} + +static int naming_sysattr_allowed(sd_device *dev, const char *sysattr) { + char *sysattr_property; + int r; + + assert(dev); + assert(sysattr); + + sysattr_property = strjoina("ID_NET_NAME_ALLOW_", sysattr); + ascii_strupper(sysattr_property); + + r = device_get_property_bool(dev, sysattr_property); + if (r == -ENOENT) + /* If ID_NET_NAME_ALLOW is not set or set to 1 default is to allow */ + return naming_sysattr_allowed_by_default(dev); + + return r; +} + +int device_get_sysattr_int_filtered(sd_device *device, const char *sysattr, int *ret_value) { + int r; + + r = naming_sysattr_allowed(device, sysattr); + if (r < 0) + return r; + if (r == 0) + return -ENOENT; + + return device_get_sysattr_int(device, sysattr, ret_value); +} + +int device_get_sysattr_unsigned_filtered(sd_device *device, const char *sysattr, unsigned *ret_value) { + int r; + + r = naming_sysattr_allowed(device, sysattr); + if (r < 0) + return r; + if (r == 0) + return -ENOENT; + + return device_get_sysattr_unsigned(device, sysattr, ret_value); +} + +int device_get_sysattr_bool_filtered(sd_device *device, const char *sysattr) { + int r; + + r = naming_sysattr_allowed(device, sysattr); + if (r < 0) + return r; + if (r == 0) + return -ENOENT; + + return device_get_sysattr_bool(device, sysattr); +} + +int device_get_sysattr_value_filtered(sd_device *device, const char *sysattr, const char **ret_value) { + int r; + + r = naming_sysattr_allowed(device, sysattr); + if (r < 0) + return r; + if (r == 0) + return -ENOENT; + + return sd_device_get_sysattr_value(device, sysattr, ret_value); +} diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h index 707c0d26f3..03dc854786 100644 --- a/src/shared/netif-naming-scheme.h +++ b/src/shared/netif-naming-scheme.h @@ -3,6 +3,8 @@ #include +#include "sd-device.h" + #include "macro.h" /* So here's the deal: net_id is supposed to be an exercise in providing stable names for network devices. However, we @@ -90,3 +92,8 @@ NamePolicy name_policy_from_string(const char *p) _pure_; const char *alternative_names_policy_to_string(NamePolicy p) _const_; NamePolicy alternative_names_policy_from_string(const char *p) _pure_; + +int device_get_sysattr_int_filtered(sd_device *device, const char *sysattr, int *ret_value); +int device_get_sysattr_unsigned_filtered(sd_device *device, const char *sysattr, unsigned *ret_value); +int device_get_sysattr_bool_filtered(sd_device *device, const char *sysattr); +int device_get_sysattr_value_filtered(sd_device *device, const char *sysattr, const char **ret_value); diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 7eaaf9530e..1997053eb9 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -145,7 +145,7 @@ static int get_dev_port(sd_device *dev, bool fallback_to_dev_id, unsigned *ret) /* Get kernel provided port index for the case when multiple ports on a single PCI function. */ - r = device_get_sysattr_unsigned(dev, "dev_port", &v); + r = device_get_sysattr_unsigned_filtered(dev, "dev_port", &v); if (r < 0) return r; if (r > 0) { @@ -161,7 +161,7 @@ static int get_dev_port(sd_device *dev, bool fallback_to_dev_id, unsigned *ret) if (fallback_to_dev_id) { unsigned iftype; - r = device_get_sysattr_unsigned(dev, "type", &iftype); + r = device_get_sysattr_unsigned_filtered(dev, "type", &iftype); if (r < 0) return r; @@ -169,7 +169,7 @@ static int get_dev_port(sd_device *dev, bool fallback_to_dev_id, unsigned *ret) } if (fallback_to_dev_id) - return device_get_sysattr_unsigned(dev, "dev_id", ret); + return device_get_sysattr_unsigned_filtered(dev, "dev_id", ret); /* Otherwise, return the original index 0. */ *ret = 0; @@ -186,7 +186,7 @@ static int get_port_specifier(sd_device *dev, bool fallback_to_dev_id, char **re assert(ret); /* First, try to use the kernel provided front panel port name for multiple port PCI device. */ - r = sd_device_get_sysattr_value(dev, "phys_port_name", &phys_port_name); + r = device_get_sysattr_value_filtered(dev, "phys_port_name", &phys_port_name); if (r >= 0 && !isempty(phys_port_name)) { if (naming_scheme_has(NAMING_SR_IOV_R)) { int vf_id = -1; @@ -248,10 +248,10 @@ static int pci_get_onboard_index(sd_device *dev, unsigned *ret) { assert(ret); /* ACPI _DSM — device specific method for naming a PCI or PCI Express device */ - r = device_get_sysattr_unsigned(dev, "acpi_index", &idx); + r = device_get_sysattr_unsigned_filtered(dev, "acpi_index", &idx); if (r < 0) /* SMBIOS type 41 — Onboard Devices Extended Information */ - r = device_get_sysattr_unsigned(dev, "index", &idx); + r = device_get_sysattr_unsigned_filtered(dev, "index", &idx); if (r < 0) return r; @@ -291,7 +291,7 @@ static int dev_pci_onboard(sd_device *dev, const LinkInfo *info, NetNames *names idx, strna(port), special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(names->pci_onboard)); - if (sd_device_get_sysattr_value(names->pcidev, "label", &names->pci_onboard_label) >= 0) + if (device_get_sysattr_value_filtered(names->pcidev, "label", &names->pci_onboard_label) >= 0) log_device_debug(dev, "Onboard label from PCI device: %s", names->pci_onboard_label); else names->pci_onboard_label = NULL; @@ -328,7 +328,7 @@ static int is_pci_multifunction(sd_device *dev) { static bool is_pci_ari_enabled(sd_device *dev) { const char *a; - if (sd_device_get_sysattr_value(dev, "ari_enabled", &a) < 0) + if (device_get_sysattr_value_filtered(dev, "ari_enabled", &a) < 0) return false; return streq(a, "1"); @@ -337,7 +337,7 @@ static bool is_pci_ari_enabled(sd_device *dev) { static bool is_pci_bridge(sd_device *dev) { const char *v, *p; - if (sd_device_get_sysattr_value(dev, "modalias", &v) < 0) + if (device_get_sysattr_value_filtered(dev, "modalias", &v) < 0) return false; if (!startswith(v, "pci:")) @@ -377,7 +377,7 @@ static int parse_hotplug_slot_from_function_id(sd_device *dev, int slots_dirfd, if (!naming_scheme_has(NAMING_SLOT_FUNCTION_ID)) return 0; - if (sd_device_get_sysattr_value(dev, "function_id", &attr) < 0) + if (device_get_sysattr_value_filtered(dev, "function_id", &attr) < 0) return 0; r = safe_atou64(attr, &function_id); @@ -438,7 +438,7 @@ static int pci_get_hotplug_slot_from_address( if (!path) return -ENOMEM; - if (sd_device_get_sysattr_value(pci, path, &address) < 0) + if (device_get_sysattr_value_filtered(pci, path, &address) < 0) continue; /* match slot address with device by stripping the function */ @@ -787,7 +787,7 @@ static int names_devicetree(sd_device *dev, const char *prefix, bool test) { if (!alias_index) continue; - if (sd_device_get_sysattr_value(aliases_dev, alias, &alias_path) < 0) + if (device_get_sysattr_value_filtered(aliases_dev, alias, &alias_path) < 0) continue; if (!path_equal(ofnode_path, alias_path)) @@ -806,7 +806,7 @@ static int names_devicetree(sd_device *dev, const char *prefix, bool test) { } /* ...but make sure we don't have an alias conflict */ - if (i == 0 && sd_device_get_sysattr_value(aliases_dev, conflict, NULL) >= 0) + if (i == 0 && device_get_sysattr_value_filtered(aliases_dev, conflict, NULL) >= 0) return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EEXIST), "Ethernet alias conflict: ethernet and ethernet0 both exist"); @@ -1077,7 +1077,7 @@ static int names_mac(sd_device *dev, const char *prefix, bool test) { assert(dev); assert(prefix); - r = device_get_sysattr_unsigned(dev, "type", &iftype); + r = device_get_sysattr_unsigned_filtered(dev, "type", &iftype); if (r < 0) return log_device_debug_errno(dev, r, "Failed to read 'type' attribute: %m"); @@ -1089,7 +1089,7 @@ static int names_mac(sd_device *dev, const char *prefix, bool test) { "Not generating MAC name for infiniband device."); /* check for NET_ADDR_PERM, skip random MAC addresses */ - r = device_get_sysattr_unsigned(dev, "addr_assign_type", &assign_type); + r = device_get_sysattr_unsigned_filtered(dev, "addr_assign_type", &assign_type); if (r < 0) return log_device_debug_errno(dev, r, "Failed to read/parse addr_assign_type: %m"); @@ -1097,7 +1097,7 @@ static int names_mac(sd_device *dev, const char *prefix, bool test) { return log_device_debug_errno(dev, SYNTHETIC_ERRNO(EINVAL), "addr_assign_type=%u, MAC address is not permanent.", assign_type); - r = sd_device_get_sysattr_value(dev, "address", &s); + r = device_get_sysattr_value_filtered(dev, "address", &s); if (r < 0) return log_device_debug_errno(dev, r, "Failed to read 'address' attribute: %m"); @@ -1147,7 +1147,7 @@ static int names_netdevsim(sd_device *dev, const char *prefix, bool test) { if (r < 0) return r; - r = sd_device_get_sysattr_value(dev, "phys_port_name", &phys_port_name); + r = device_get_sysattr_value_filtered(dev, "phys_port_name", &phys_port_name); if (r < 0) return r; if (isempty(phys_port_name)) @@ -1227,7 +1227,7 @@ static int get_ifname_prefix(sd_device *dev, const char **ret) { assert(dev); assert(ret); - r = device_get_sysattr_unsigned(dev, "type", &iftype); + r = device_get_sysattr_unsigned_filtered(dev, "type", &iftype); if (r < 0) return r; @@ -1274,7 +1274,7 @@ static int get_link_info(sd_device *dev, LinkInfo *info) { if (r < 0) return r; - r = device_get_sysattr_int(dev, "iflink", &info->iflink); + r = device_get_sysattr_int_filtered(dev, "iflink", &info->iflink); if (r < 0) return r; -- 2.43.0