[systemd-commits] 3 commits - src/core src/journal src/libsystemd-terminal src/random-seed src/shared src/systemctl src/tty-ask-password-agent src/vconsole
Zbigniew Jędrzejewski-Szmek
zbyszek at kemper.freedesktop.org
Tue Dec 9 18:51:51 PST 2014
src/core/ima-setup.c | 66 +++++---------------
src/core/machine-id-setup.c | 5 -
src/journal/compress.c | 11 ---
src/journal/journal-send.c | 5 -
src/journal/journalctl.c | 14 +---
src/libsystemd-terminal/subterm.c | 8 +-
src/random-seed/random-seed.c | 15 +---
src/shared/copy.c | 11 +--
src/shared/util.c | 9 +-
src/shared/util.h | 2
src/systemctl/systemctl.c | 31 ++-------
src/tty-ask-password-agent/tty-ask-password-agent.c | 12 +--
src/vconsole/vconsole-setup.c | 10 +--
13 files changed, 69 insertions(+), 130 deletions(-)
New commits:
commit a644abed54bd4a42ebe2c99af5cc621ffbaf6c55
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Tue Dec 9 14:41:24 2014 -0500
systemctl: fix invalid free when enabling sysv services fails
The error was introduced in v215-343-g60731f32f1 'systemctl: do not
bother to mutate state on error', by causing strv_free to attempt to
free a static string. Simplify the whole thing by always keeping the
array in valid state.
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 6e48671..17dfff7 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -5143,7 +5143,7 @@ static int enable_sysv_units(const char *verb, char **args) {
int r = 0;
#if defined(HAVE_SYSV_COMPAT) && defined(HAVE_CHKCONFIG)
- unsigned f = 1, t = 1;
+ unsigned f = 0;
_cleanup_lookup_paths_free_ LookupPaths paths = {};
if (arg_scope != UNIT_FILE_SYSTEM)
@@ -5162,7 +5162,7 @@ static int enable_sysv_units(const char *verb, char **args) {
return r;
r = 0;
- for (f = 0; args[f]; f++) {
+ while (args[f]) {
const char *name;
_cleanup_free_ char *p = NULL, *q = NULL, *l = NULL;
bool found_native = false, found_sysv;
@@ -5173,7 +5173,7 @@ static int enable_sysv_units(const char *verb, char **args) {
pid_t pid;
siginfo_t status;
- name = args[f];
+ name = args[f++];
if (!endswith(name, ".service"))
continue;
@@ -5205,9 +5205,6 @@ static int enable_sysv_units(const char *verb, char **args) {
if (!found_sysv)
continue;
- /* Mark this entry, so that we don't try enabling it as native unit */
- args[f] = (char*) "";
-
log_info("%s is not a native service, redirecting to /sbin/chkconfig.", name);
if (!isempty(arg_root))
@@ -5256,19 +5253,12 @@ static int enable_sysv_units(const char *verb, char **args) {
return -EINVAL;
} else
return -EPROTO;
- }
-
- /* Drop all SysV units */
- for (f = 0, t = 0; args[f]; f++) {
- if (isempty(args[f]))
- continue;
-
- args[t++] = args[f];
+ /* Remove this entry, so that we don't try enabling it as native unit */
+ assert(f > 0 && streq(args[f-1], name));
+ assert_se(strv_remove(args + f - 1, name));
}
- args[t] = NULL;
-
#endif
return r;
}
commit 4dfb18922d5d1efb13ee459cbf23832277f85ed7
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Mon Dec 1 20:47:37 2014 -0500
ima-setup: simplify
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 3470ca1..8e4fed1 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -24,18 +24,14 @@
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
-#include <string.h>
-#include <stdlib.h>
-#include <fcntl.h>
+#include <sys/types.h>
#include <sys/stat.h>
-#include <sys/mman.h>
+#include <fcntl.h>
#include "ima-setup.h"
-#include "mount-setup.h"
-#include "macro.h"
+#include "copy.h"
#include "util.h"
#include "log.h"
-#include "label.h"
#define IMA_SECFS_DIR "/sys/kernel/security/ima"
#define IMA_SECFS_POLICY IMA_SECFS_DIR "/policy"
@@ -45,58 +41,37 @@ int ima_setup(void) {
int r = 0;
#ifdef HAVE_IMA
- struct stat st;
- ssize_t policy_size = 0;
- char *policy;
_cleanup_close_ int policyfd = -1, imafd = -1;
- if (stat(IMA_POLICY_PATH, &st) < 0)
- return 0;
-
- policy_size = st.st_size;
- if (stat(IMA_SECFS_DIR, &st) < 0) {
+ if (access(IMA_SECFS_DIR, F_OK) < 0) {
log_debug("IMA support is disabled in the kernel, ignoring.");
return 0;
}
- if (stat(IMA_SECFS_POLICY, &st) < 0) {
- log_error("Another IMA custom policy has already been loaded, ignoring.");
+ policyfd = open(IMA_POLICY_PATH, O_RDONLY|O_CLOEXEC);
+ if (policyfd < 0) {
+ log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno,
+ "Failed to open the IMA custom policy file "IMA_POLICY_PATH", ignoring: %m");
return 0;
}
- policyfd = open(IMA_POLICY_PATH, O_RDONLY|O_CLOEXEC);
- if (policyfd < 0) {
- log_error_errno(errno, "Failed to open the IMA custom policy file %s (%m), ignoring.",
- IMA_POLICY_PATH);
+ if (access(IMA_SECFS_POLICY, F_OK) < 0) {
+ log_warning("Another IMA custom policy has already been loaded, ignoring.");
return 0;
}
imafd = open(IMA_SECFS_POLICY, O_WRONLY|O_CLOEXEC);
if (imafd < 0) {
- log_error_errno(errno, "Failed to open the IMA kernel interface %s (%m), ignoring.",
- IMA_SECFS_POLICY);
- goto out;
- }
-
- policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0);
- if (policy == MAP_FAILED) {
- log_error_errno(errno, "mmap() failed (%m), freezing");
- r = -errno;
- goto out;
+ log_error_errno(errno, "Failed to open the IMA kernel interface "IMA_SECFS_POLICY", ignoring: %m");
+ return 0;
}
- r = loop_write(imafd, policy, (size_t)policy_size, false);
- if (r < 0) {
- log_error_errno(r, "Failed to load the IMA custom policy file %s (%m), ignoring.",
- IMA_POLICY_PATH);
- goto out_mmap;
- }
+ r = copy_bytes(policyfd, imafd, -1);
+ if (r < 0)
+ log_error_errno(r, "Failed to load the IMA custom policy file "IMA_POLICY_PATH": %m");
+ else
+ log_info("Successfully loaded the IMA custom policy "IMA_POLICY_PATH".");
- log_info("Successfully loaded the IMA custom policy %s.",
- IMA_POLICY_PATH);
-out_mmap:
- munmap(policy, policy_size);
-out:
#endif /* HAVE_IMA */
return r;
}
commit 553acb7b6b8d4f16a4747b1f978e8b7888fbfb2c
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date: Mon Dec 1 20:43:19 2014 -0500
treewide: sanitize loop_write
loop_write() didn't follow the usual systemd rules and returned status
partially in errno and required extensive checks from callers. Some of
the callers dealt with this properly, but many did not, treating
partial writes as successful. Simplify things by conforming to usual rules.
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 3416802..3470ca1 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -42,13 +42,13 @@
#define IMA_POLICY_PATH "/etc/ima/ima-policy"
int ima_setup(void) {
+ int r = 0;
#ifdef HAVE_IMA
struct stat st;
- ssize_t policy_size = 0, written = 0;
+ ssize_t policy_size = 0;
char *policy;
_cleanup_close_ int policyfd = -1, imafd = -1;
- int result = 0;
if (stat(IMA_POLICY_PATH, &st) < 0)
return 0;
@@ -81,13 +81,13 @@ int ima_setup(void) {
policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0);
if (policy == MAP_FAILED) {
log_error_errno(errno, "mmap() failed (%m), freezing");
- result = -errno;
+ r = -errno;
goto out;
}
- written = loop_write(imafd, policy, (size_t)policy_size, false);
- if (written != policy_size) {
- log_error_errno(errno, "Failed to load the IMA custom policy file %s (%m), ignoring.",
+ r = loop_write(imafd, policy, (size_t)policy_size, false);
+ if (r < 0) {
+ log_error_errno(r, "Failed to load the IMA custom policy file %s (%m), ignoring.",
IMA_POLICY_PATH);
goto out_mmap;
}
@@ -97,9 +97,6 @@ int ima_setup(void) {
out_mmap:
munmap(policy, policy_size);
out:
- if (result)
- return result;
#endif /* HAVE_IMA */
-
- return 0;
+ return r;
}
diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index 74582a5..d91a02c 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -182,7 +182,7 @@ static int write_machine_id(int fd, char id[34]) {
assert(id);
lseek(fd, 0, SEEK_SET);
- if (loop_write(fd, id, 33, false) == 33)
+ if (loop_write(fd, id, 33, false) == 0)
return 0;
return -errno;
@@ -329,10 +329,9 @@ int machine_id_setup(const char *root) {
if (r < 0)
return r;
- if (S_ISREG(st.st_mode) && writable) {
+ if (S_ISREG(st.st_mode) && writable)
if (write_machine_id(fd, id) == 0)
return 0;
- }
fd = safe_close(fd);
diff --git a/src/journal/compress.c b/src/journal/compress.c
index c4c715b..9440fcd 100644
--- a/src/journal/compress.c
+++ b/src/journal/compress.c
@@ -400,12 +400,9 @@ int compress_stream_xz(int fdf, int fdt, off_t max_bytes) {
n = sizeof(out) - s.avail_out;
- errno = 0;
k = loop_write(fdt, out, n, false);
if (k < 0)
return k;
- if (k != n)
- return errno ? -errno : -EIO;
if (ret == LZMA_STREAM_END) {
log_debug("XZ compression finished (%"PRIu64" -> %"PRIu64" bytes, %.1f%%)",
@@ -478,8 +475,6 @@ int compress_stream_lz4(int fdf, int fdt, off_t max_bytes) {
n = loop_write(fdt, out, r, false);
if (n < 0)
return n;
- if (n != r)
- return errno ? -errno : -EIO;
total_out += sizeof(header) + r;
@@ -559,12 +554,9 @@ int decompress_stream_xz(int fdf, int fdt, off_t max_bytes) {
max_bytes -= n;
}
- errno = 0;
k = loop_write(fdt, out, n, false);
if (k < 0)
return k;
- if (k != n)
- return errno ? -errno : -EIO;
if (ret == LZMA_STREAM_END) {
log_debug("XZ decompression finished (%"PRIu64" -> %"PRIu64" bytes, %.1f%%)",
@@ -645,12 +637,9 @@ int decompress_stream_lz4(int fdf, int fdt, off_t max_bytes) {
return -EFBIG;
}
- errno = 0;
n = loop_write(fdt, out, r, false);
if (n < 0)
return n;
- if (n != r)
- return errno ? -errno : -EIO;
}
log_debug("LZ4 decompression finished (%zu -> %zu bytes, %.1f%%)",
diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
index 887b957c..56a96c5 100644
--- a/src/journal/journal-send.c
+++ b/src/journal/journal-send.c
@@ -453,13 +453,10 @@ _public_ int sd_journal_stream_fd(const char *identifier, int priority, int leve
header[l++] = '0';
header[l++] = '\n';
- r = (int) loop_write(fd, header, l, false);
+ r = loop_write(fd, header, l, false);
if (r < 0)
return r;
- if ((size_t) r != l)
- return -errno;
-
r = fd;
fd = -1;
return r;
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 3cec9a0..b2f6966 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1406,17 +1406,15 @@ static int setup_keys(void) {
h.fsprg_secpar = htole16(FSPRG_RECOMMENDED_SECPAR);
h.fsprg_state_size = htole64(state_size);
- l = loop_write(fd, &h, sizeof(h), false);
- if (l < 0 || (size_t) l != sizeof(h)) {
- log_error_errno(EIO, "Failed to write header: %m");
- r = -EIO;
+ r = loop_write(fd, &h, sizeof(h), false);
+ if (r < 0) {
+ log_error_errno(r, "Failed to write header: %m");
goto finish;
}
- l = loop_write(fd, state, state_size, false);
- if (l < 0 || (size_t) l != state_size) {
- log_error_errno(EIO, "Failed to write state: %m");
- r = -EIO;
+ r = loop_write(fd, state, state_size, false);
+ if (r < 0) {
+ log_error_errno(r, "Failed to write state: %m");
goto finish;
}
diff --git a/src/libsystemd-terminal/subterm.c b/src/libsystemd-terminal/subterm.c
index 75a25e5..78efc9d 100644
--- a/src/libsystemd-terminal/subterm.c
+++ b/src/libsystemd-terminal/subterm.c
@@ -117,14 +117,14 @@ static int output_winch(Output *o) {
}
static int output_flush(Output *o) {
- ssize_t len;
+ int r;
if (o->n_obuf < 1)
return 0;
- len = loop_write(o->fd, o->obuf, o->n_obuf, false);
- if (len < 0)
- return log_error_errno(len, "error: cannot write to TTY (%zd): %m", len);
+ r = loop_write(o->fd, o->obuf, o->n_obuf, false);
+ if (r < 0)
+ return log_error_errno(r, "error: cannot write to TTY: %m");
o->n_obuf = 0;
diff --git a/src/random-seed/random-seed.c b/src/random-seed/random-seed.c
index 40eaaf4..06c1239 100644
--- a/src/random-seed/random-seed.c
+++ b/src/random-seed/random-seed.c
@@ -113,12 +113,9 @@ int main(int argc, char *argv[]) {
} else {
lseek(seed_fd, 0, SEEK_SET);
- k = loop_write(random_fd, buf, (size_t) k, false);
- if (k <= 0) {
- log_error("Failed to write seed to /dev/urandom: %s", r < 0 ? strerror(-r) : "short write");
-
- r = k == 0 ? -EIO : (int) k;
- }
+ r = loop_write(random_fd, buf, (size_t) k, false);
+ if (r < 0)
+ log_error_errno(r, "Failed to write seed to /dev/urandom: %m");
}
} else if (streq(argv[1], "save")) {
@@ -155,10 +152,8 @@ int main(int argc, char *argv[]) {
r = k == 0 ? -EIO : (int) k;
} else {
r = loop_write(seed_fd, buf, (size_t) k, false);
- if (r <= 0) {
- log_error("Failed to write new random seed file: %s", r < 0 ? strerror(-r) : "short write");
- r = r == 0 ? -EIO : r;
- }
+ if (r < 0)
+ log_error_errno(r, "Failed to write new random seed file: %m");
}
finish:
diff --git a/src/shared/copy.c b/src/shared/copy.c
index abb7fbc..b8b1ba1 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -63,7 +63,7 @@ int copy_bytes(int fdf, int fdt, off_t max_bytes) {
/* As a fallback just copy bits by hand */
{
char buf[m];
- ssize_t k;
+ int r;
n = read(fdf, buf, m);
if (n < 0)
@@ -71,12 +71,9 @@ int copy_bytes(int fdf, int fdt, off_t max_bytes) {
if (n == 0) /* EOF */
break;
- errno = 0;
- k = loop_write(fdt, buf, n, false);
- if (k < 0)
- return k;
- if (k != n)
- return errno ? -errno : -EIO;
+ r = loop_write(fdt, buf, n, false);
+ if (r < 0)
+ return r;
}
diff --git a/src/shared/util.c b/src/shared/util.c
index ff8835b..26a4f72 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -2292,13 +2292,15 @@ ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll) {
return n;
}
-ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
+int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
const uint8_t *p = buf;
ssize_t n = 0;
assert(fd >= 0);
assert(buf);
+ errno = 0;
+
while (nbytes > 0) {
ssize_t k;
@@ -2317,14 +2319,15 @@ ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
}
if (k <= 0)
- return n > 0 ? n : (k < 0 ? -errno : 0);
+ /* We were not done yet, and a write error occured. */
+ return errno ? -errno : -EIO;
p += k;
nbytes -= k;
n += k;
}
- return n;
+ return 0;
}
int parse_size(const char *t, off_t base, off_t *size) {
diff --git a/src/shared/util.h b/src/shared/util.h
index 61094cc..73bd901 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -425,7 +425,7 @@ int sigaction_many(const struct sigaction *sa, ...);
int fopen_temporary(const char *path, FILE **_f, char **_temp_path);
ssize_t loop_read(int fd, void *buf, size_t nbytes, bool do_poll);
-ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll);
+int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll);
bool is_device_path(const char *path);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index d356686..6e48671 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -7246,12 +7246,9 @@ static int talk_initctl(void) {
return -errno;
}
- errno = 0;
- r = loop_write(fd, &request, sizeof(request), false) != sizeof(request);
- if (r) {
- log_error_errno(errno, "Failed to write to "INIT_FIFO": %m");
- return errno > 0 ? -errno : -EIO;
- }
+ r = loop_write(fd, &request, sizeof(request), false);
+ if (r < 0)
+ return log_error_errno(r, "Failed to write to "INIT_FIFO": %m");
return 1;
}
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index fa8448c..5fc27f9 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -103,9 +103,9 @@ static int ask_password_plymouth(
if (!packet)
return log_oom();
- k = loop_write(fd, packet, n + 1, true);
- if (k != n + 1)
- return k < 0 ? (int) k : -EIO;
+ r = loop_write(fd, packet, n + 1, true);
+ if (r < 0)
+ return r;
pollfd[POLL_SOCKET].fd = fd;
pollfd[POLL_SOCKET].events = POLLIN;
@@ -165,9 +165,9 @@ static int ask_password_plymouth(
if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0)
return -ENOMEM;
- k = loop_write(fd, packet, n+1, true);
- if (k != n + 1)
- return k < 0 ? (int) k : -EIO;
+ r = loop_write(fd, packet, n+1, true);
+ if (r < 0)
+ return r;
accept_cached = false;
p = 0;
diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c
index b7a536b..2837171 100644
--- a/src/vconsole/vconsole-setup.c
+++ b/src/vconsole/vconsole-setup.c
@@ -54,8 +54,9 @@ static int disable_utf8(int fd) {
if (ioctl(fd, KDSKBMODE, K_XLATE) < 0)
r = -errno;
- if (loop_write(fd, "\033%@", 3, false) < 0)
- r = -errno;
+ k = loop_write(fd, "\033%@", 3, false);
+ if (k < 0)
+ r = k;
k = write_string_file("/sys/module/vt/parameters/default_utf8", "0");
if (k < 0)
@@ -86,8 +87,9 @@ static int enable_utf8(int fd) {
r = -errno;
}
- if (loop_write(fd, "\033%G", 3, false) < 0)
- r = -errno;
+ k = loop_write(fd, "\033%G", 3, false);
+ if (k < 0)
+ r = k;
k = write_string_file("/sys/module/vt/parameters/default_utf8", "1");
if (k < 0)
More information about the systemd-commits
mailing list