sd-device: replace lstat() + open() with open(O_NOFOLLOW)

Coverity was complaining about TOCTOU (CID #745806). Indeed, it seems better
to open the file and avoid the stat altogether:

- O_NOFOLLOW means we'll get ELOOP, which we can translate to EINVAL as before,
- similarly, open(O_WRONLY) on a directory will fail with EISDIR,
- and finally, it makes no sense to check access mode ourselves: just let
  the kernel do it and propagate the error.

v2:
- fix memleak, don't clober input arg

diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
index efeadf0cd4ba1219e5ee02401c620b2fba68835c..04ead29338bbe88298ac888370b9dd8fddd12cc2 100644 (file)
--- a/src/libsystemd/sd-device/sd-device.c
+++ b/src/libsystemd/sd-device/sd-device.c
@@ -1859,8 +1859,7 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
         _cleanup_free_ char *value = NULL;
         const char *syspath;
         char *path;
-        struct stat statbuf;
-        size_t value_len = 0;
+        size_t len = 0;
         ssize_t size;
         int r;
 
@@ -1878,8 +1877,14 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
                 return r;
 
         path = strjoina(syspath, "/", sysattr);
-        r = lstat(path, &statbuf);
-        if (r < 0) {
+
+        fd = open(path, O_WRONLY | O_CLOEXEC | O_NOFOLLOW);
+        if (fd < 0) {
+                if (errno == ELOOP)
+                        return -EINVAL;
+                if (errno == EISDIR)
+                        return -EISDIR;
+
                 value = strdup("");
                 if (!value)
                         return -ENOMEM;
@@ -1891,46 +1896,30 @@ _public_ int sd_device_set_sysattr_value(sd_device *device, const char *sysattr,
                 return -ENXIO;
         }
 
-        if (S_ISLNK(statbuf.st_mode))
-                return -EINVAL;
-
-        /* skip directories */
-        if (S_ISDIR(statbuf.st_mode))
-                return -EISDIR;
-
-        /* skip non-readable files */
-        if ((statbuf.st_mode & S_IRUSR) == 0)
-                return -EACCES;
-
-        value_len = strlen(_value);
+        len = strlen(_value);
 
         /* drop trailing newlines */
-        while (value_len > 0 && _value[value_len - 1] == '\n')
-                _value[--value_len] = '\0';
+        while (len > 0 && _value[len - 1] == '\n')
+                len --;
 
         /* value length is limited to 4k */
-        if (value_len > 4096)
+        if (len > 4096)
                 return -EINVAL;
 
-        fd = open(path, O_WRONLY | O_CLOEXEC);
-        if (fd < 0)
-                return -errno;
-
-        value = strdup(_value);
+        value = strndup(_value, len);
         if (!value)
                 return -ENOMEM;
 
-        size = write(fd, value, value_len);
+        size = write(fd, value, len);
         if (size < 0)
                 return -errno;
 
-        if ((size_t)size != value_len)
+        if ((size_t)size != len)
                 return -EIO;
 
         r = device_add_sysattr_value(device, sysattr, value);
         if (r < 0)
                 return r;
-
         value = NULL;
 
         return 0;