aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--TODO27
-rw-r--r--src/basic/fileio.c108
-rw-r--r--src/basic/fileio.h16
-rw-r--r--src/basic/process-util.c62
-rw-r--r--src/basic/terminal-util.c13
-rw-r--r--src/machine/machine.c12
-rw-r--r--src/machine/machined-dbus.c8
-rw-r--r--src/test/test-fileio.c67
8 files changed, 177 insertions, 136 deletions
diff --git a/TODO b/TODO
index b7038ec7c..1fbffc740 100644
--- a/TODO
+++ b/TODO
@@ -34,8 +34,6 @@ Features:
* when importing an fs tree with machined, complain if image is not an OS
-* when we fork off generators and such, lower LIMIT_NOFILE soft limit to 1K
-
* Maybe introduce a helper safe_exec() or so, which is to execve() which
safe_fork() is to fork(). And then make revert the RLIMIT_NOFILE soft limit
to 1K implicitly, unless explicitly opted-out.
@@ -63,16 +61,11 @@ Features:
* when a socket unit is spawned with an AF_UNIX path in /var/run, complain and
patch it to use /run instead
-* consider splitting out all temporary file creation APIs (we have so many in
- fileio.h and elsewhere!) into a new util file of its own.
-
* set memory.oom.group in cgroupsv2 for all leaf cgroups (kernel v4.19+)
* add a new syscall group "@esoteric" for more esoteric stuff such as bpf() and
usefaultd() and make systemd-analyze check for it.
-* drop umask() calls and suchlike from our generators, pid1 should set things up correctly anyway
-
* paranoia: whenever we process passwords, call mlock() on the memory
first. i.e. look for all places we use string_erase()/string_free_erase() and
augment them with mlock()
@@ -165,13 +158,6 @@ Features:
* nspawn: greater control over selinux label?
-* cgroups: figure out if we can somehow communicate in a cleaner way whether a
- systemd instance not running in the cgroup root shall or shall not manage the
- attributes of its top-level cgroup. Currently it assumes it manages all, but
- then might get EPERM due to permission porblems/userns, which is OK, but this
- should be revisited to make clearer and also work if the payload systemd runs
- with full privs and without userns.
-
* hibernate/s2h: make this robust and safe to enable in Fedora by default.
Specifically:
@@ -320,11 +306,6 @@ Features:
StateDirectory=, LogsDirectory=, CacheDirectory=, as well as RootDirectory= if it
is set, plus the whole disk space any image configured with RootImage=.
-* Introduce "exit" as an EmergencyAction value, and allow to configure a
- per-unit success/failure exit code to configure. This would be useful for
- running commands inside of services inside of containers, which could then
- propagate their failure state all the way up.
-
* In DynamicUser= mode: before selecting a UID, use disk quota APIs on relevant
disks to see if the UID is already in use.
@@ -341,7 +322,7 @@ Features:
* show whether a service has out-of-date configuration in "systemctl status" by
using mtime data of ConfigurationDirectory=.
-* replace all uses of fgets() + LINE_MAX by read_line()
+* replace all remaining uses of fgets() + LINE_MAX by read_line()
* Add AddUser= setting to unit files, similar to DynamicUser=1 which however
creates a static, persistent user rather than a dynamic, transient user. We
@@ -507,9 +488,6 @@ Features:
state.
http://lists.freedesktop.org/archives/systemd-devel/2015-April/030229.html
-* Maybe add support for the equivalent of "ethtool advertise" to .link files?
- http://lists.freedesktop.org/archives/systemd-devel/2015-April/030112.html
-
* The udev blkid built-in should expose a property that reflects
whether media was sensed in USB CF/SD card readers. This should then
be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
@@ -635,7 +613,6 @@ Features:
* cgroups:
- implement per-slice CPUFairScheduling=1 switch
- - handle jointly mounted controllers correctly
- introduce high-level settings for RT budget, swappiness
- how to reset dynamically changed unit cgroup attributes sanely?
- when reloading configuration, apply new cgroup configuration
@@ -904,8 +881,6 @@ Features:
PID 1...
- optionally automatically add FORWARD rules to iptables whenever nspawn is
running, remove them when shut down.
- - maybe make copying of /etc/resolv.conf optional, and skip it if --read-only
- is used
* dissect
- refuse mounting over a mount point
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index b22fcd030..66e127345 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -667,46 +667,6 @@ int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space)
return fputs(s, f);
}
-int read_nul_string(FILE *f, char **ret) {
- _cleanup_free_ char *x = NULL;
- size_t allocated = 0, n = 0;
-
- assert(f);
- assert(ret);
-
- /* Reads a NUL-terminated string from the specified file. */
-
- for (;;) {
- int c;
-
- if (!GREEDY_REALLOC(x, allocated, n+2))
- return -ENOMEM;
-
- c = fgetc(f);
- if (c == 0) /* Terminate at NUL byte */
- break;
- if (c == EOF) {
- if (ferror(f))
- return -errno;
- break; /* Terminate at EOF */
- }
-
- x[n++] = (char) c;
- }
-
- if (x)
- x[n] = 0;
- else {
- x = new0(char, 1);
- if (!x)
- return -ENOMEM;
- }
-
- *ret = TAKE_PTR(x);
-
- return 0;
-}
-
/* A bitmask of the EOL markers we know */
typedef enum EndOfLineMarker {
EOL_NONE = 0,
@@ -715,11 +675,15 @@ typedef enum EndOfLineMarker {
EOL_THIRTEEN = 1 << 2, /* \r (aka CR) */
} EndOfLineMarker;
-static EndOfLineMarker categorize_eol(char c) {
- if (c == '\n')
- return EOL_TEN;
- if (c == '\r')
- return EOL_THIRTEEN;
+static EndOfLineMarker categorize_eol(char c, ReadLineFlags flags) {
+
+ if (!IN_SET(flags, READ_LINE_ONLY_NUL)) {
+ if (c == '\n')
+ return EOL_TEN;
+ if (c == '\r')
+ return EOL_THIRTEEN;
+ }
+
if (c == '\0')
return EOL_ZERO;
@@ -728,9 +692,10 @@ static EndOfLineMarker categorize_eol(char c) {
DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, funlockfile);
-int read_line(FILE *f, size_t limit, char **ret) {
+int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
size_t n = 0, allocated = 0, count = 0;
_cleanup_free_ char *buffer = NULL;
+ int r;
assert(f);
@@ -770,7 +735,7 @@ int read_line(FILE *f, size_t limit, char **ret) {
for (;;) {
EndOfLineMarker eol;
- int c;
+ char c;
if (n >= limit)
return -ENOBUFS;
@@ -778,20 +743,13 @@ int read_line(FILE *f, size_t limit, char **ret) {
if (count >= INT_MAX) /* We couldn't return the counter anymore as "int", hence refuse this */
return -ENOBUFS;
- errno = 0;
- c = fgetc_unlocked(f);
- if (c == EOF) {
- /* if we read an error, and have no data to return, then propagate the error */
- if (ferror_unlocked(f) && n == 0)
- return errno > 0 ? -errno : -EIO;
-
- /* EOF is line ending too. */
+ r = safe_fgetc(f, &c);
+ if (r < 0)
+ return r;
+ if (r == 0) /* EOF is definitely EOL */
break;
- }
-
- count++;
- eol = categorize_eol(c);
+ eol = categorize_eol(c, flags);
if (FLAGS_SET(previous_eol, EOL_ZERO) ||
(eol == EOL_NONE && previous_eol != EOL_NONE) ||
@@ -800,10 +758,11 @@ int read_line(FILE *f, size_t limit, char **ret) {
* EOL marker has been seen right before? In either of these three cases we are
* done. But first, let's put this character back in the queue. */
assert_se(ungetc(c, f) != EOF);
- count--;
break;
}
+ count++;
+
if (eol != EOL_NONE) {
previous_eol |= eol;
continue;
@@ -813,7 +772,7 @@ int read_line(FILE *f, size_t limit, char **ret) {
if (!GREEDY_REALLOC(buffer, allocated, n + 2))
return -ENOMEM;
- buffer[n] = (char) c;
+ buffer[n] = c;
}
n++;
@@ -828,3 +787,30 @@ int read_line(FILE *f, size_t limit, char **ret) {
return (int) count;
}
+
+int safe_fgetc(FILE *f, char *ret) {
+ int k;
+
+ assert(f);
+
+ /* A safer version of plain fgetc(): let's propagate the error that happened while reading as such, and
+ * separate the EOF condition from the byte read, to avoid those confusion signed/unsigned issues fgetc()
+ * has. */
+
+ errno = 0;
+ k = fgetc(f);
+ if (k == EOF) {
+ if (ferror(f))
+ return errno > 0 ? -errno : -EIO;
+
+ if (ret)
+ *ret = 0;
+
+ return 0;
+ }
+
+ if (ret)
+ *ret = k;
+
+ return 1;
+}
diff --git a/src/basic/fileio.h b/src/basic/fileio.h
index 848024607..53e3f4ef5 100644
--- a/src/basic/fileio.h
+++ b/src/basic/fileio.h
@@ -61,6 +61,18 @@ int read_timestamp_file(const char *fn, usec_t *ret);
int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space);
-int read_nul_string(FILE *f, char **ret);
+typedef enum ReadLineFlags {
+ READ_LINE_ONLY_NUL = 1 << 0,
+} ReadLineFlags;
-int read_line(FILE *f, size_t limit, char **ret);
+int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret);
+
+static inline int read_line(FILE *f, size_t limit, char **ret) {
+ return read_line_full(f, limit, 0, ret);
+}
+
+static inline int read_nul_string(FILE *f, size_t limit, char **ret) {
+ return read_line_full(f, limit, READ_LINE_ONLY_NUL, ret);
+}
+
+int safe_fgetc(FILE *f, char *ret);
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index 5b700df33..3cfaceea8 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -600,12 +600,14 @@ int get_process_root(pid_t pid, char **root) {
return get_process_link_contents(p, root);
}
+#define ENVIRONMENT_BLOCK_MAX (5U*1024U*1024U)
+
int get_process_environ(pid_t pid, char **env) {
_cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *outcome = NULL;
- int c;
- const char *p;
size_t allocated = 0, sz = 0;
+ const char *p;
+ int r;
assert(pid >= 0);
assert(env);
@@ -621,23 +623,28 @@ int get_process_environ(pid_t pid, char **env) {
(void) __fsetlocking(f, FSETLOCKING_BYCALLER);
- while ((c = fgetc(f)) != EOF) {
+ for (;;) {
+ char c;
+
+ if (sz >= ENVIRONMENT_BLOCK_MAX)
+ return -ENOBUFS;
+
if (!GREEDY_REALLOC(outcome, allocated, sz + 5))
return -ENOMEM;
+ r = safe_fgetc(f, &c);
+ if (r < 0)
+ return r;
+ if (r == 0)
+ break;
+
if (c == '\0')
outcome[sz++] = '\n';
else
sz += cescape_char(c, outcome + sz);
}
- if (!outcome) {
- outcome = strdup("");
- if (!outcome)
- return -ENOMEM;
- } else
- outcome[sz] = '\0';
-
+ outcome[sz] = '\0';
*env = TAKE_PTR(outcome);
return 0;
@@ -870,9 +877,9 @@ int kill_and_sigcont(pid_t pid, int sig) {
int getenv_for_pid(pid_t pid, const char *field, char **ret) {
_cleanup_fclose_ FILE *f = NULL;
char *value = NULL;
- bool done = false;
const char *path;
- size_t l;
+ size_t l, sum = 0;
+ int r;
assert(pid >= 0);
assert(field);
@@ -895,6 +902,9 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) {
return 1;
}
+ if (!pid_is_valid(pid))
+ return -EINVAL;
+
path = procfs_file_alloca(pid, "environ");
f = fopen(path, "re");
@@ -908,24 +918,19 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) {
(void) __fsetlocking(f, FSETLOCKING_BYCALLER);
l = strlen(field);
+ for (;;) {
+ _cleanup_free_ char *line = NULL;
- do {
- char line[LINE_MAX];
- size_t i;
-
- for (i = 0; i < sizeof(line)-1; i++) {
- int c;
+ if (sum > ENVIRONMENT_BLOCK_MAX) /* Give up searching eventually */
+ return -ENOBUFS;
- c = getc(f);
- if (_unlikely_(c == EOF)) {
- done = true;
- break;
- } else if (c == 0)
- break;
+ r = read_nul_string(f, LONG_LINE_MAX, &line);
+ if (r < 0)
+ return r;
+ if (r == 0) /* EOF */
+ break;
- line[i] = c;
- }
- line[i] = 0;
+ sum += r;
if (strneq(line, field, l) && line[l] == '=') {
value = strdup(line + l + 1);
@@ -935,8 +940,7 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) {
*ret = value;
return 1;
}
-
- } while (!done);
+ }
*ret = NULL;
return 0;
diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index ceba7d0ff..0f3812072 100644
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -96,7 +96,7 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
new_termios.c_cc[VTIME] = 0;
if (tcsetattr(fileno(f), TCSADRAIN, &new_termios) >= 0) {
- int c;
+ char c;
if (t != USEC_INFINITY) {
if (fd_wait_for_event(fileno(f), POLLIN, t) <= 0) {
@@ -105,17 +105,12 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) {
}
}
- errno = 0;
- c = fgetc(f);
- if (c == EOF)
- r = ferror(f) && errno > 0 ? -errno : -EIO;
- else
- r = 0;
-
+ r = safe_fgetc(f, &c);
(void) tcsetattr(fileno(f), TCSADRAIN, &old_termios);
-
if (r < 0)
return r;
+ if (r == 0)
+ return -EIO;
if (need_nl)
*need_nl = c != '\n';
diff --git a/src/machine/machine.c b/src/machine/machine.c
index beb5b3566..4f89ac026 100644
--- a/src/machine/machine.c
+++ b/src/machine/machine.c
@@ -594,7 +594,7 @@ int machine_get_uid_shift(Machine *m, uid_t *ret) {
uid_t uid_base, uid_shift, uid_range;
gid_t gid_base, gid_shift, gid_range;
_cleanup_fclose_ FILE *f = NULL;
- int k;
+ int k, r;
assert(m);
assert(ret);
@@ -643,7 +643,10 @@ int machine_get_uid_shift(Machine *m, uid_t *ret) {
return -ENXIO;
/* If there's more than one line, then we don't support this mapping. */
- if (fgetc(f) != EOF)
+ r = safe_fgetc(f, NULL);
+ if (r < 0)
+ return r;
+ if (r != 0) /* Insist on EOF */
return -ENXIO;
fclose(f);
@@ -664,7 +667,10 @@ int machine_get_uid_shift(Machine *m, uid_t *ret) {
}
/* If there's more than one line, then we don't support this file. */
- if (fgetc(f) != EOF)
+ r = safe_fgetc(f, NULL);
+ if (r < 0)
+ return r;
+ if (r != 0) /* Insist on EOF */
return -ENXIO;
/* If the UID and GID mapping doesn't match, we don't support this mapping. */
diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c
index 9afae72ae..fea9cc263 100644
--- a/src/machine/machined-dbus.c
+++ b/src/machine/machined-dbus.c
@@ -637,8 +637,8 @@ static int clean_pool_done(Operation *operation, int ret, sd_bus_error *error) {
if (success) /* The resulting temporary file could not be updated, ignore it. */
return ret;
- r = read_nul_string(f, &name);
- if (r < 0 || isempty(name)) /* Same here... */
+ r = read_nul_string(f, LONG_LINE_MAX, &name);
+ if (r <= 0) /* Same here... */
return ret;
return sd_bus_error_set_errnof(error, ret, "Failed to remove image %s: %m", name);
@@ -660,10 +660,10 @@ static int clean_pool_done(Operation *operation, int ret, sd_bus_error *error) {
_cleanup_free_ char *name = NULL;
uint64_t size;
- r = read_nul_string(f, &name);
+ r = read_nul_string(f, LONG_LINE_MAX, &name);
if (r < 0)
return r;
- if (isempty(name)) /* reached the end */
+ if (r == 0) /* reached the end */
break;
errno = 0;
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index fd0cc27de..f75b75757 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -610,9 +610,36 @@ static void test_tempfn(void) {
free(ret);
}
+static const char chars[] =
+ "Aąę„”\n루";
+
+static void test_fgetc(void) {
+ _cleanup_fclose_ FILE *f = NULL;
+ char c;
+
+ f = fmemopen((void*) chars, sizeof(chars), "re");
+ assert_se(f);
+
+ for (unsigned i = 0; i < sizeof(chars); i++) {
+ assert_se(safe_fgetc(f, &c) == 1);
+ assert_se(c == chars[i]);
+
+ assert_se(ungetc(c, f) != EOF);
+ assert_se(safe_fgetc(f, &c) == 1);
+ assert_se(c == chars[i]);
+
+ /* Check that ungetc doesn't care about unsigned char vs signed char */
+ assert_se(ungetc((unsigned char) c, f) != EOF);
+ assert_se(safe_fgetc(f, &c) == 1);
+ assert_se(c == chars[i]);
+ }
+
+ assert_se(safe_fgetc(f, &c) == 0);
+}
+
static const char buffer[] =
"Some test data\n"
- "Some weird line\r"
+ "루Non-ascii chars: ąę„”\n"
"terminators\r\n"
"and even more\n\r"
"now the same with a NUL\n\0"
@@ -631,7 +658,7 @@ static void test_read_line_one_file(FILE *f) {
assert_se(read_line(f, (size_t) -1, &line) == 15 && streq(line, "Some test data"));
line = mfree(line);
- assert_se(read_line(f, (size_t) -1, &line) == 16 && streq(line, "Some weird line"));
+ assert_se(read_line(f, (size_t) -1, &line) > 0 && streq(line, "루Non-ascii chars: ąę„”"));
line = mfree(line);
assert_se(read_line(f, (size_t) -1, &line) == 13 && streq(line, "terminators"));
@@ -748,6 +775,40 @@ static void test_read_line4(void) {
}
}
+static void test_read_nul_string(void) {
+ static const char test[] = "string nr. 1\0"
+ "string nr. 2\n\0"
+ "empty string follows\0"
+ "\0"
+ "final string\n is empty\0"
+ "\0";
+
+ _cleanup_fclose_ FILE *f = NULL;
+ _cleanup_free_ char *s = NULL;
+
+ assert_se(f = fmemopen((void*) test, sizeof(test)-1, "r"));
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 13 && streq_ptr(s, "string nr. 1"));
+ s = mfree(s);
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 14 && streq_ptr(s, "string nr. 2\n"));
+ s = mfree(s);
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 21 && streq_ptr(s, "empty string follows"));
+ s = mfree(s);
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 1 && streq_ptr(s, ""));
+ s = mfree(s);
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 23 && streq_ptr(s, "final string\n is empty"));
+ s = mfree(s);
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 1 && streq_ptr(s, ""));
+ s = mfree(s);
+
+ assert_se(read_nul_string(f, LONG_LINE_MAX, &s) == 0 && streq_ptr(s, ""));
+}
+
int main(int argc, char *argv[]) {
test_setup_logging(LOG_DEBUG);
@@ -767,10 +828,12 @@ int main(int argc, char *argv[]) {
test_search_and_fopen_nulstr();
test_writing_tmpfile();
test_tempfn();
+ test_fgetc();
test_read_line();
test_read_line2();
test_read_line3();
test_read_line4();
+ test_read_nul_string();
return 0;
}