]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
sd-device: ignore error in device_cache_sysattr_value() and propagate original error...
authorYu Watanabe <watanabe.yu+github@gmail.com>
Thu, 18 Feb 2021 14:22:27 +0000 (23:22 +0900)
committerYu Watanabe <watanabe.yu+github@gmail.com>
Sat, 20 Feb 2021 19:40:17 +0000 (04:40 +0900)
There are three calls of device_cache_sysattr_value(). Two of them are
just caching the value. Hence, let's ignore its failure, and propagate
original error code.

One exception is the last call in sd_device_get_sysattr_value().
Unfortunately, it returns `const char *` instead of `char *`. So,
sd_device object must have the reference of the returned value.
Hence, error in updating the cache by device_cache_sysattr_value()
is critical, and we need to propagate the error in that case.

src/libsystemd/sd-device/sd-device.c
src/libsystemd/sd-device/test-sd-device.c
src/shared/dissect-image.c
src/shared/udev-util.c

index 991779aa9c15766334aa4bdcbb5cf42e9c75e9f5..dad87f39c9a82259ac7ccd42b4f777e18dcf54f7 100644 (file)
@@ -1894,12 +1894,16 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
         path = prefix_roota(syspath, sysattr);
         r = lstat(path, &statbuf);
         if (r < 0) {
+                int k;
+
                 /* remember that we could not access the sysattr */
-                r = device_cache_sysattr_value(device, sysattr, NULL);
-                if (r < 0)
-                        return r;
+                k = device_cache_sysattr_value(device, sysattr, NULL);
+                if (k < 0)
+                        log_device_debug_errno(device, k,
+                                               "sd-device: failed to cache attribute '%s' with NULL, ignoring: %m",
+                                               sysattr);
 
-                return -ENOENT;
+                return r;
         } else if (S_ISLNK(statbuf.st_mode)) {
                 /* Some core links return only the last element of the target path,
                  * these are just values, the paths should not be exposed. */
@@ -1925,11 +1929,16 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
                 delete_trailing_chars(value, "\n");
         }
 
+        /* Unfortunately, we need to return 'const char*' instead of 'char*'. Hence, failure in caching
+         * sysattr value is critical unlike the other places. */
         r = device_cache_sysattr_value(device, sysattr, value);
-        if (r < 0)
-                return r;
-
-        if (ret_value)
+        if (r < 0) {
+                log_device_debug_errno(device, r,
+                                       "sd-device: failed to cache attribute '%s' with '%s'%s: %m",
+                                       sysattr, value, ret_value ? "" : ", ignoring");
+                if (ret_value)
+                        return r;
+        } else if (ret_value)
                 *ret_value = TAKE_PTR(value);
 
         return 0;
@@ -1990,8 +1999,11 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
 
         r = device_cache_sysattr_value(device, sysattr, value);
         if (r < 0)
-                return r;
-        TAKE_PTR(value);
+                log_device_debug_errno(device, r,
+                                       "sd-device: failed to cache attribute '%s' with '%s', ignoring: %m",
+                                       sysattr, value);
+        else
+                TAKE_PTR(value);
 
         return 0;
 }
index fd30d1a1d023ad3b741086f7e3f2125724e5ed85..aaa16f740d68f2b966ee6e8e37824789452b6b7c 100644 (file)
@@ -4,6 +4,7 @@
 #include "device-internal.h"
 #include "device-private.h"
 #include "device-util.h"
+#include "errno-util.h"
 #include "hashmap.h"
 #include "nulstr-util.h"
 #include "string-util.h"
@@ -51,7 +52,7 @@ static void test_sd_device_one(sd_device *d) {
         }
 
         r = sd_device_get_sysattr_value(d, "name_assign_type", &val);
-        assert_se(r >= 0 || IN_SET(r, -ENOENT, -EINVAL));
+        assert_se(r >= 0 || ERRNO_IS_PRIVILEGE(r) || IN_SET(r, -ENOENT, -EINVAL));
 
         r = sd_device_get_property_value(d, "ID_NET_DRIVER", &val);
         assert_se(r >= 0 || r == -ENOENT);
index 6d24c3fd9ff149a9dac5e3acc0e92bcefb30f2fb..8de1738264539fbba2ed9eac0f7042043680fbdb 100644 (file)
@@ -150,7 +150,8 @@ static int device_is_partition(sd_device *d, blkid_partition pp) {
                 return false;
 
         r = sd_device_get_sysattr_value(d, "partition", &v);
-        if (r == -ENOENT) /* Not a partition device */
+        if (r == -ENOENT ||        /* Not a partition device */
+            ERRNO_IS_PRIVILEGE(r)) /* Not ready to access? */
                 return false;
         if (r < 0)
                 return r;
index 3a3f0198fdb20778d8060987255fc27e75d58a8a..80b70c18a0377b33a8cefd5587f632f6dd5cc3c4 100644 (file)
@@ -10,6 +10,7 @@
 #include "device-private.h"
 #include "device-util.h"
 #include "env-file.h"
+#include "errno-util.h"
 #include "escape.h"
 #include "fd-util.h"
 #include "log.h"
@@ -525,12 +526,12 @@ int udev_resolve_subsys_kernel(const char *string, char *result, size_t maxsize,
 
         if (read_value) {
                 r = sd_device_get_sysattr_value(dev, attr, &val);
-                if (r < 0 && r != -ENOENT)
+                if (r < 0 && !ERRNO_IS_PRIVILEGE(r) && r != -ENOENT)
                         return r;
-                if (r == -ENOENT)
-                        result[0] = '\0';
-                else
+                if (r >= 0)
                         strscpy(result, maxsize, val);
+                else
+                        result[0] = '\0';
                 log_debug("value '[%s/%s]%s' is '%s'", subsys, sysname, attr, result);
         } else {
                 r = sd_device_get_syspath(dev, &val);