107 lines
3.5 KiB
Diff
107 lines
3.5 KiB
Diff
From a7c2ac892ad0c6e9d270bf9adf071f0aab456282 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
|
Date: Sun, 19 Feb 2017 14:17:19 -0500
|
|
Subject: [PATCH] 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
|
|
|
|
(cherry picked from commit 2fa4861ad5a203bff604cac660136834e3b70108)
|
|
---
|
|
src/libsystemd/sd-device/sd-device.c | 43 ++++++++++++++----------------------
|
|
1 file changed, 16 insertions(+), 27 deletions(-)
|
|
|
|
diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
|
|
index 0c4ad966bd..1d68fe07ae 100644
|
|
--- a/src/libsystemd/sd-device/sd-device.c
|
|
+++ b/src/libsystemd/sd-device/sd-device.c
|
|
@@ -1857,8 +1857,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;
|
|
|
|
@@ -1876,8 +1875,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;
|
|
@@ -1889,46 +1894,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;
|