From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Wed, 11 Mar 2026 22:00:06 +0000 (+0900)
Subject: tree-wide: use device_get_sysattr_safe_string()
X-Git-Tag: v261-rc1~125^2~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=06d687ffacecac22cbe5a99723e99bf0401757a1;p=thirdparty%2Fsystemd.git

tree-wide: use device_get_sysattr_safe_string()

The obtained strings are passed to another function, e,g, handled as a
path and opened, printed to the terminal, written to a file, saved to
udev database as udev property, exposed through DBus, passed to logger,
and so on. Hence, these should not contain any malicious characters.
---

diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
index 7e296c9ffcb..29ce29fab91 100644
--- a/src/backlight/backlight.c
+++ b/src/backlight/backlight.c
@@ -281,7 +281,7 @@ static int validate_device(sd_device *device) {
                                 const char *other_sysname = NULL, *other_type = NULL;
 
                                 (void) sd_device_get_sysname(other, &other_sysname);
-                                (void) sd_device_get_sysattr_value(other, "type", &other_type);
+                                (void) device_get_sysattr_safe_string(other, "type", &other_type);
                                 log_device_debug(device,
                                                  "Found another %s backlight device %s on the same PCI, skipping.",
                                                  strna(other_type), strna(other_sysname));
@@ -295,7 +295,7 @@ static int validate_device(sd_device *device) {
                                 const char *other_sysname = NULL, *other_type = NULL;
 
                                 (void) sd_device_get_sysname(other, &other_sysname);
-                                (void) sd_device_get_sysattr_value(other, "type", &other_type);
+                                (void) device_get_sysattr_safe_string(other, "type", &other_type);
                                 log_device_debug(device,
                                                  "Found another %s backlight device %s, which has higher precedence, skipping.",
                                                  strna(other_type), strna(other_sysname));
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index 5ec7b2fea99..2292c076613 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -20,6 +20,7 @@
 #include "constants.h"
 #include "daemon-util.h"
 #include "device-private.h"
+#include "device-util.h"
 #include "env-file.h"
 #include "env-util.h"
 #include "extract-word.h"
@@ -442,12 +443,14 @@ static int get_sysattr(sd_device *device, const char *key, char **ret) {
         if (!device)
                 return -ENODEV;
 
-        r = sd_device_get_sysattr_value(device, key, &s);
+        r = device_get_sysattr_safe_string(device, key, &s);
         if (r < 0)
-                return r;
+                return log_device_debug_errno(device, r, "Failed to read '%s' attribute: %m", key);
 
         if (!string_is_safe_for_dbus(s))
-                return -ENXIO;
+                return log_device_debug_errno(device, SYNTHETIC_ERRNO(ENXIO),
+                                              "'%s' attribute is not safe for exposing through DBus: %s",
+                                              key, s);
 
         return strdup_to(ret, empty_to_null(s));
 }
@@ -703,7 +706,7 @@ static const char* fallback_chassis_by_device_tree(Context *c) {
         if (!c->device_tree)
                 return NULL;
 
-        r = sd_device_get_sysattr_value(c->device_tree, "chassis-type", &type);
+        r = device_get_sysattr_safe_string(c->device_tree, "chassis-type", &type);
         if (r < 0) {
                 log_debug_errno(r, "Failed to read device-tree chassis type, ignoring: %m");
                 return NULL;
diff --git a/src/login/sysfs-show.c b/src/login/sysfs-show.c
index 202041832c4..bdbd8a64e44 100644
--- a/src/login/sysfs-show.c
+++ b/src/login/sysfs-show.c
@@ -4,6 +4,7 @@
 
 #include "alloc-util.h"
 #include "device-enumerator-private.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "glyph-util.h"
 #include "path-util.h"
@@ -61,8 +62,8 @@ static int show_sysfs_one(
 
                 is_master = sd_device_has_current_tag(dev_list[*i_dev], "master-of-seat") > 0;
 
-                if (sd_device_get_sysattr_value(dev_list[*i_dev], "name", &name) < 0)
-                        (void) sd_device_get_sysattr_value(dev_list[*i_dev], "id", &name);
+                if (device_get_sysattr_safe_string(dev_list[*i_dev], "name", &name) < 0)
+                        (void) device_get_sysattr_safe_string(dev_list[*i_dev], "id", &name);
 
                 /* Look if there's more coming after this */
                 for (lookahead = *i_dev + 1; lookahead < n_dev; lookahead++) {
diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c
index 636930d2771..2e391ee4d24 100644
--- a/src/mount/mount-tool.c
+++ b/src/mount/mount-tool.c
@@ -12,6 +12,7 @@
 #include "bus-util.h"
 #include "bus-wait-for-jobs.h"
 #include "chase.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "errno-util.h"
 #include "escape.h"
@@ -840,7 +841,7 @@ static int find_loop_device(const char *backing_file, sd_device **ret) {
         FOREACH_DEVICE(e, dev) {
                 const char *s;
 
-                r = sd_device_get_sysattr_value(dev, "loop/backing_file", &s);
+                r = device_get_sysattr_safe_string(dev, "loop/backing_file", &s);
                 if (r < 0) {
                         log_device_debug_errno(dev, r, "Failed to read \"loop/backing_file\" sysattr, ignoring: %m");
                         continue;
diff --git a/src/network/networkd-sriov.c b/src/network/networkd-sriov.c
index f5591f5672d..46644259791 100644
--- a/src/network/networkd-sriov.c
+++ b/src/network/networkd-sriov.c
@@ -3,6 +3,7 @@
 
 #include "sd-netlink.h"
 
+#include "device-private.h"
 #include "device-util.h"
 #include "errno-util.h"
 #include "hashmap.h"
@@ -282,7 +283,7 @@ int link_set_sr_iov_ifindices(Link *link) {
 
         /* This may return -EINVAL or -ENODEV, instead of -ENOENT, if the device has been removed or is being
          * removed. Let's ignore the error codes here. */
-        r = sd_device_get_sysattr_value(link->dev, "dev_port", &dev_port);
+        r = device_get_sysattr_safe_string(link->dev, "dev_port", &dev_port);
         if (ERRNO_IS_NEG_DEVICE_ABSENT(r) || r == -EINVAL)
                 return 0;
         if (r < 0)
diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c
index f5b67e5f01c..36ae1d8d234 100644
--- a/src/rfkill/rfkill.c
+++ b/src/rfkill/rfkill.c
@@ -10,6 +10,7 @@
 #include "sd-device.h"
 
 #include "alloc-util.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "errno-util.h"
 #include "escape.h"
@@ -82,7 +83,7 @@ static int find_device(
                 return log_full_errno(ERRNO_IS_DEVICE_ABSENT(r) ? LOG_DEBUG : LOG_ERR, r,
                                       "Failed to open device '%s': %m", sysname);
 
-        r = sd_device_get_sysattr_value(device, "name", &name);
+        r = device_get_sysattr_safe_string(device, "name", &name);
         if (r < 0)
                 return log_device_debug_errno(device, r, "Device has no name, ignoring: %m");
 
diff --git a/src/shared/battery-util.c b/src/shared/battery-util.c
index 8ca9a6d4b2b..7c3336f4654 100644
--- a/src/shared/battery-util.c
+++ b/src/shared/battery-util.c
@@ -43,7 +43,7 @@ static int device_is_power_sink(sd_device *device) {
         FOREACH_DEVICE(e, d) {
                 const char *val;
 
-                r = sd_device_get_sysattr_value(d, "power_role", &val);
+                r = device_get_sysattr_safe_string(d, "power_role", &val);
                 if (r < 0) {
                         if (r != -ENOENT)
                                 log_device_debug_errno(d, r, "Failed to read 'power_role' sysfs attribute, ignoring: %m");
@@ -93,7 +93,7 @@ static bool battery_is_discharging(sd_device *d) {
         }
 
         /* Possible values: "Unknown", "Charging", "Discharging", "Not charging", "Full" */
-        r = sd_device_get_sysattr_value(d, "status", &val);
+        r = device_get_sysattr_safe_string(d, "status", &val);
         if (r < 0) {
                 log_device_debug_errno(d, r, "Failed to read 'status' sysfs attribute, assuming the battery is discharging: %m");
                 return true;
@@ -130,7 +130,7 @@ int on_ac_power(void) {
                  * https://docs.kernel.org/admin-guide/abi-testing.html#abi-file-testing-sysfs-class-power */
 
                 const char *val;
-                r = sd_device_get_sysattr_value(d, "type", &val);
+                r = device_get_sysattr_safe_string(d, "type", &val);
                 if (r < 0) {
                         log_device_debug_errno(d, r, "Failed to read 'type' sysfs attribute, ignoring device: %m");
                         continue;
diff --git a/src/shared/loop-util.c b/src/shared/loop-util.c
index 3437afcda49..1debc4267f4 100644
--- a/src/shared/loop-util.c
+++ b/src/shared/loop-util.c
@@ -15,6 +15,7 @@
 #include "alloc-util.h"
 #include "blockdev-util.h"
 #include "data-fd-util.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "devnum-util.h"
 #include "dissect-image.h"
@@ -1027,7 +1028,7 @@ int loop_device_open(
 #endif
                 nr = info.lo_number;
 
-                if (sd_device_get_sysattr_value(dev, "loop/backing_file", &s) >= 0) {
+                if (device_get_sysattr_safe_string(dev, "loop/backing_file", &s) >= 0) {
                         backing_file = strdup(s);
                         if (!backing_file)
                                 return -ENOMEM;
diff --git a/src/udev/udev-builtin-hwdb.c b/src/udev/udev-builtin-hwdb.c
index dececd9c037..5cda0f9efd4 100644
--- a/src/udev/udev-builtin-hwdb.c
+++ b/src/udev/udev-builtin-hwdb.c
@@ -6,6 +6,7 @@
 #include "sd-hwdb.h"
 
 #include "alloc-util.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "hwdb-util.h"
 #include "options.h"
@@ -59,7 +60,7 @@ static const char* modalias_usb(sd_device *dev, char *s, size_t size) {
                 return NULL;
         if (safe_atoux16(p, &pn) < 0)
                 return NULL;
-        (void) sd_device_get_sysattr_value(dev, "product", &n);
+        (void) device_get_sysattr_safe_string(dev, "product", &n);
 
         (void) snprintf(s, size, "usb:v%04Xp%04X:%s", vn, pn, strempty(n));
         return s;
diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index ecb2afe0488..da4a9b33a45 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -9,6 +9,7 @@
 #include <fcntl.h>
 #include <linux/input.h>
 
+#include "device-private.h"
 #include "device-util.h"
 #include "fd-util.h"
 #include "parse-util.h"
@@ -85,7 +86,7 @@ static void get_cap_mask(
         unsigned long val;
         int r;
 
-        if (sd_device_get_sysattr_value(pdev, attr, &v) < 0)
+        if (device_get_sysattr_safe_string(pdev, attr, &v) < 0)
                 v = "";
 
         xsprintf(text, "%s", v);
diff --git a/src/udev/udev-builtin-keyboard.c b/src/udev/udev-builtin-keyboard.c
index 3ced8ad91ca..926ee50b3e5 100644
--- a/src/udev/udev-builtin-keyboard.c
+++ b/src/udev/udev-builtin-keyboard.c
@@ -5,6 +5,7 @@
 #include <stdlib.h>
 #include <sys/ioctl.h>
 
+#include "device-private.h"
 #include "device-util.h"
 #include "errno-util.h"
 #include "fd-util.h"
@@ -32,7 +33,7 @@ static int install_force_release(sd_device *dev, const unsigned *release, unsign
         if (r < 0)
                 return log_device_error_errno(dev, r, "Failed to get serio parent: %m");
 
-        r = sd_device_get_sysattr_value(atkbd, "force_release", &cur);
+        r = device_get_sysattr_safe_string(atkbd, "force_release", &cur);
         if (r < 0)
                 return log_device_error_errno(atkbd, r, "Failed to get force-release attribute: %m");
 
diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c
index 6d6108ee01b..af4e2b2d40d 100644
--- a/src/udev/udev-builtin-path_id.c
+++ b/src/udev/udev-builtin-path_id.c
@@ -108,7 +108,7 @@ static sd_device* handle_scsi_fibre_channel(sd_device *parent, char **path) {
                 return NULL;
         if (sd_device_new_from_subsystem_sysname(&fcdev, "fc_transport", sysname) < 0)
                 return NULL;
-        if (sd_device_get_sysattr_value(fcdev, "port_name", &port) < 0)
+        if (device_get_sysattr_safe_string(fcdev, "port_name", &port) < 0)
                 return NULL;
 
         format_lun_number(parent, &lun);
@@ -133,7 +133,7 @@ static sd_device* handle_scsi_sas_wide_port(sd_device *parent, char **path) {
                 return NULL;
         if (sd_device_new_from_subsystem_sysname(&sasdev, "sas_device", sysname) < 0)
                 return NULL;
-        if (sd_device_get_sysattr_value(sasdev, "sas_address", &sas_address) < 0)
+        if (device_get_sysattr_safe_string(sasdev, "sas_address", &sas_address) < 0)
                 return NULL;
 
         format_lun_number(parent, &lun);
@@ -175,7 +175,7 @@ static sd_device* handle_scsi_sas(sd_device *parent, char **path) {
                 return handle_scsi_sas_wide_port(parent, path);
 
         /* Get connected phy */
-        if (sd_device_get_sysattr_value(target_sasdev, "phy_identifier", &phy_id) < 0)
+        if (device_get_sysattr_safe_string(target_sasdev, "phy_identifier", &phy_id) < 0)
                 return NULL;
 
         /* The port's parent is either hba or expander */
@@ -187,7 +187,7 @@ static sd_device* handle_scsi_sas(sd_device *parent, char **path) {
         /* Get expander device */
         if (sd_device_new_from_subsystem_sysname(&expander_sasdev, "sas_device", sysname) >= 0) {
                 /* Get expander's address */
-                if (sd_device_get_sysattr_value(expander_sasdev, "sas_address", &sas_address) < 0)
+                if (device_get_sysattr_safe_string(expander_sasdev, "sas_address", &sas_address) < 0)
                         return NULL;
         }
 
@@ -224,7 +224,7 @@ static sd_device* handle_scsi_iscsi(sd_device *parent, char **path) {
         if (sd_device_new_from_subsystem_sysname(&sessiondev, "iscsi_session", sysname) < 0)
                 return NULL;
 
-        if (sd_device_get_sysattr_value(sessiondev, "targetname", &target) < 0)
+        if (device_get_sysattr_safe_string(sessiondev, "targetname", &target) < 0)
                 return NULL;
 
         if (sd_device_get_sysnum(transportdev, &sysnum) < 0)
@@ -233,9 +233,9 @@ static sd_device* handle_scsi_iscsi(sd_device *parent, char **path) {
         if (sd_device_new_from_subsystem_sysname(&conndev, "iscsi_connection", connname) < 0)
                 return NULL;
 
-        if (sd_device_get_sysattr_value(conndev, "persistent_address", &addr) < 0)
+        if (device_get_sysattr_safe_string(conndev, "persistent_address", &addr) < 0)
                 return NULL;
-        if (sd_device_get_sysattr_value(conndev, "persistent_port", &port) < 0)
+        if (device_get_sysattr_safe_string(conndev, "persistent_port", &port) < 0)
                 return NULL;
 
         format_lun_number(parent, &lun);
@@ -268,7 +268,7 @@ static sd_device* handle_scsi_ata(sd_device *parent, char **path, char **compat_
         if (sd_device_new_from_subsystem_sysname(&atadev, "ata_port", sysname) < 0)
                 return NULL;
 
-        if (sd_device_get_sysattr_value(atadev, "port_no", &port_no) < 0)
+        if (device_get_sysattr_safe_string(atadev, "port_no", &port_no) < 0)
                 return NULL;
 
         if (bus != 0)
@@ -375,7 +375,7 @@ static sd_device* handle_scsi_hyperv(sd_device *parent, char **path, size_t guid
         if (sd_device_get_parent(hostdev, &vmbusdev) < 0)
                 return NULL;
 
-        if (sd_device_get_sysattr_value(vmbusdev, "device_id", &guid_str) < 0)
+        if (device_get_sysattr_safe_string(vmbusdev, "device_id", &guid_str) < 0)
                 return NULL;
 
         if (strlen(guid_str) < guid_str_len || guid_str[0] != '{' || guid_str[guid_str_len-1] != '}')
@@ -403,7 +403,7 @@ static sd_device* handle_scsi(sd_device *parent, char **path, char **compat_path
                 return parent;
 
         /* firewire */
-        if (sd_device_get_sysattr_value(parent, "ieee1394_id", &id) >= 0) {
+        if (device_get_sysattr_safe_string(parent, "ieee1394_id", &id) >= 0) {
                 path_prepend(path, "ieee1394-0x%s", id);
                 *supported_parent = true;
                 return skip_subsystem(parent, "scsi");
@@ -570,8 +570,8 @@ static sd_device* handle_ap(sd_device *parent, char **path) {
         assert(parent);
         assert(path);
 
-        if (sd_device_get_sysattr_value(parent, "type", &type) >= 0 &&
-            sd_device_get_sysattr_value(parent, "ap_functions", &func) >= 0)
+        if (device_get_sysattr_safe_string(parent, "type", &type) >= 0 &&
+            device_get_sysattr_safe_string(parent, "ap_functions", &func) >= 0)
                 path_prepend(path, "ap-%s-%s", type, func);
         else {
                 const char *sysname;
@@ -787,7 +787,7 @@ static int builtin_path_id(UdevEvent *event, int argc, char *argv[]) {
                 } else if (device_in_subsystem(parent, "nvme", "nvme-subsystem") > 0) {
                         const char *nsid;
 
-                        if (sd_device_get_sysattr_value(dev, "nsid", &nsid) >= 0) {
+                        if (device_get_sysattr_safe_string(dev, "nsid", &nsid) >= 0) {
                                 path_prepend(&path, "nvme-%s", nsid);
                                 if (compat_path)
                                         path_prepend(&compat_path, "nvme-%s", nsid);
diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index cfbea9d9819..68bc55ba9e0 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -11,6 +11,7 @@
 #include <unistd.h>
 
 #include "device-nodes.h"
+#include "device-private.h"
 #include "device-util.h"
 #include "fd-util.h"
 #include "parse-util.h"
@@ -259,8 +260,8 @@ static int builtin_usb_id(UdevEvent *event, int argc, char *argv[]) {
         r = sd_device_get_syspath(dev_interface, &interface_syspath);
         if (r < 0)
                 return log_device_debug_errno(dev_interface, r, "Failed to get syspath: %m");
-        (void) sd_device_get_sysattr_value(dev_interface, "bInterfaceNumber", &ifnum);
-        (void) sd_device_get_sysattr_value(dev_interface, "driver", &driver);
+        (void) device_get_sysattr_safe_string(dev_interface, "bInterfaceNumber", &ifnum);
+        (void) device_get_sysattr_safe_string(dev_interface, "driver", &driver);
 
         r = sd_device_get_sysattr_value(dev_interface, "bInterfaceClass", &if_class);
         if (r < 0)
diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c
index 73ed70f4bff..f2944b02f31 100644
--- a/src/udev/udevadm-info.c
+++ b/src/udev/udevadm-info.c
@@ -162,7 +162,7 @@ static int print_all_attributes(sd_device *device, bool is_parent) {
                 if (skip_attribute(name))
                         continue;
 
-                r = sd_device_get_sysattr_value(device, name, &value);
+                r = device_get_sysattr_safe_string(device, name, &value);
                 if (r >= 0) {
                         /* skip any values that look like a path */
                         if (value[0] == '/')
@@ -264,7 +264,7 @@ static int print_all_attributes_in_json(sd_device *device, bool is_parent) {
                 if (skip_attribute(name))
                         continue;
 
-                r = sd_device_get_sysattr_value(device, name, &value);
+                r = device_get_sysattr_safe_string(device, name, &value);
                 if (r >= 0) {
                         /* skip any values that look like a path */
                         if (value[0] == '/')