[systemd-devel] [RFC PATCH 1/2] Replace mkostemp+unlink with open(O_TMPFILE)
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Sat Jan 25 21:21:17 PST 2014
This will only work on Linux >= 3.11, and probably not on all
filesystems. Fallback code is provided.
---
Hi,
because on bug https://bugzilla.gnome.org/show_bug.cgi?id=722889, I
was looking into async signal safety of the journal logging functions.
All that do any formatting are unsafe, but sd_journal_sendv *almost*
is. Currently it calls mkostemp and writev, but only in the fallback
path. So for the purpose of simple logging without multi-megabyte
messages it already is safe. But it would be nice to turn this into an
explicit guarantee. When O_TMPFILE is not available, it is hard to
make it generally safe, so I thought it would nice to provide a mode
where it simply fails when safety cannot be guaranteed. Because
sd_j_sendv does not take any flags, this is done by changing the
length parameter to mean that negative means signal-safe. So on
newer kernels, everything should just work and the function should be
async-signal-safe when a negative parameter is used. On old kernels,
it would cleanly fail for large messages. I think this is a worthy goal,
and being able to call structured logging from signal handlers should
lead to better diagnostics in the long run.
Comments? It would be great if somebody could look over sd_j_sendv to
verify my assumptions that everything in it is signal safe apart from
the stuff mentioned above.
Zbyszek
src/core/manager.c | 17 +++--------------
src/journal/journal-send.c | 20 +++++++-------------
src/journal/journal-verify.c | 12 +++---------
src/shared/util.c | 22 ++++++++++++++++++++++
src/shared/util.h | 2 ++
5 files changed, 37 insertions(+), 36 deletions(-)
diff --git a/src/core/manager.c b/src/core/manager.c
index 95fc7e6..7156c38 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1985,28 +1985,17 @@ void manager_dispatch_bus_name_owner_changed(
}
int manager_open_serialization(Manager *m, FILE **_f) {
- _cleanup_free_ char *path = NULL;
+ const char *path;
int fd = -1;
FILE *f;
assert(_f);
- if (m->running_as == SYSTEMD_SYSTEM)
- asprintf(&path, "/run/systemd/dump-"PID_FMT"-XXXXXX", getpid());
- else
- asprintf(&path, "/tmp/systemd-dump-"PID_FMT"-XXXXXX", getpid());
-
- if (!path)
- return -ENOMEM;
-
- RUN_WITH_UMASK(0077) {
- fd = mkostemp(path, O_RDWR|O_CLOEXEC);
- }
-
+ path = m->running_as == SYSTEMD_SYSTEM ? "/run/systemd" : "/tmp";
+ fd = open_tmpfile(path, O_RDWR|O_CLOEXEC);
if (fd < 0)
return -errno;
- unlink(path);
log_debug("Serializing state to %s", path);
f = fdopen(fd, "w+");
diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
index 281e154..ca9199f 100644
--- a/src/journal/journal-send.c
+++ b/src/journal/journal-send.c
@@ -216,10 +216,6 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
uint8_t buf[CMSG_SPACE(sizeof(int))];
} control;
struct cmsghdr *cmsg;
- /* We use /dev/shm instead of /tmp here, since we want this to
- * be a tmpfs, and one that is available from early boot on
- * and where unprivileged users can create files. */
- char path[] = "/dev/shm/journal.XXXXXX";
bool have_syslog_identifier = false;
assert_return(iov, -EINVAL);
@@ -309,16 +305,14 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
/* Message doesn't fit... Let's dump the data in a temporary
* file and just pass a file descriptor of it to the other
- * side */
-
- buffer_fd = mkostemp(path, O_CLOEXEC|O_RDWR);
+ * side.
+ *
+ * We use /dev/shm instead of /tmp here, since we want this to
+ * be a tmpfs, and one that is available from early boot on
+ * and where unprivileged users can create files. */
+ buffer_fd = open_tmpfile("/dev/shm", O_RDWR | O_CLOEXEC);
if (buffer_fd < 0)
- return -errno;
-
- if (unlink(path) < 0) {
- close_nointr_nofail(buffer_fd);
- return -errno;
- }
+ return buffer_fd;
n = writev(buffer_fd, w, j);
if (n < 0) {
diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c
index f2422ff..9434cc9 100644
--- a/src/journal/journal-verify.c
+++ b/src/journal/journal-verify.c
@@ -785,9 +785,6 @@ int journal_file_verify(
uint64_t n_weird = 0, n_objects = 0, n_entries = 0, n_data = 0, n_fields = 0, n_data_hash_tables = 0, n_field_hash_tables = 0, n_entry_arrays = 0, n_tags = 0;
usec_t last_usec = 0;
int data_fd = -1, entry_fd = -1, entry_array_fd = -1;
- char data_path[] = "/var/tmp/journal-data-XXXXXX",
- entry_path[] = "/var/tmp/journal-entry-XXXXXX",
- entry_array_path[] = "/var/tmp/journal-entry-array-XXXXXX";
unsigned i;
bool found_last;
#ifdef HAVE_GCRYPT
@@ -808,29 +805,26 @@ int journal_file_verify(
} else if (f->seal)
return -ENOKEY;
- data_fd = mkostemp(data_path, O_CLOEXEC);
+ data_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC);
if (data_fd < 0) {
log_error("Failed to create data file: %m");
r = -errno;
goto fail;
}
- unlink(data_path);
- entry_fd = mkostemp(entry_path, O_CLOEXEC);
+ entry_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC);
if (entry_fd < 0) {
log_error("Failed to create entry file: %m");
r = -errno;
goto fail;
}
- unlink(entry_path);
- entry_array_fd = mkostemp(entry_array_path, O_CLOEXEC);
+ entry_array_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC);
if (entry_array_fd < 0) {
log_error("Failed to create entry array file: %m");
r = -errno;
goto fail;
}
- unlink(entry_array_path);
#ifdef HAVE_GCRYPT
if ((le32toh(f->header->compatible_flags) & ~HEADER_COMPATIBLE_SEALED) != 0)
diff --git a/src/shared/util.c b/src/shared/util.c
index 5551714..34b3d96 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -6105,3 +6105,25 @@ int getpeersec(int fd, char **ret) {
*ret = s;
return 0;
}
+
+int open_tmpfile(const char *path, int flags) {
+ int fd;
+ char *p;
+
+#ifdef O_TMPFILE
+ fd = open(path, flags | O_TMPFILE, S_IRUSR | S_IWUSR);
+ if (fd >= 0)
+ return fd;
+#endif
+ p = strappenda(path, "/systemd-tmp-XXXXXX");
+
+ RUN_WITH_UMASK(0077) {
+ fd = mkostemp(p, O_RDWR|O_CLOEXEC);
+ }
+
+ if (fd < 0)
+ return -errno;
+
+ unlink(path);
+ return fd;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index d6d746b..630137a 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -849,3 +849,5 @@ bool pid_valid(pid_t pid);
int getpeercred(int fd, struct ucred *ucred);
int getpeersec(int fd, char **ret);
+
+int open_tmpfile(const char *path, int flags);
--
1.8.5.3
More information about the systemd-devel
mailing list