summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2015-11-13 00:54:56 +0100
committerLennart Poettering <lennart@poettering.net>2015-11-13 13:02:49 +0100
commiteb3da9012f462da2451edeb8d67c5b67c833a0b1 (patch)
treec009896e29778ebe5ac2fdcb140f5fdb27aa2788
parent87fde73e185fabc346ee4d9c9befe972e3502dc3 (diff)
util-lib: optionally, when writing a string to a file, verify string on failure
With this change, the idiom: r = write_string_file(p, buf, 0); if (r < 0) { if (verify_one_line_file(p, buf) > 0) r = 0; } gets reduced to: r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); i.e. when writing the string fails and the new flag WRITE_STRING_FILE_VERIFY_ON_FAILURE is specified we'll not return a failure immediately, but check the contents of the file. If it matches what we wanted to write we suppress the error and exit cleanly.
-rw-r--r--src/basic/fileio.c83
-rw-r--r--src/basic/fileio.h3
-rw-r--r--src/network/networkd-link.c54
-rw-r--r--src/test/test-fileio.c21
4 files changed, 104 insertions, 57 deletions
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index be6e327690..10aacdc56d 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -78,6 +78,7 @@ static int write_string_file_atomic(const char *fn, const char *line, bool enfor
int write_string_file(const char *fn, const char *line, WriteStringFileFlags flags) {
_cleanup_fclose_ FILE *f = NULL;
+ int q, r;
assert(fn);
assert(line);
@@ -85,30 +86,58 @@ int write_string_file(const char *fn, const char *line, WriteStringFileFlags fla
if (flags & WRITE_STRING_FILE_ATOMIC) {
assert(flags & WRITE_STRING_FILE_CREATE);
- return write_string_file_atomic(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE));
+ r = write_string_file_atomic(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE));
+ if (r < 0)
+ goto fail;
+
+ return r;
}
if (flags & WRITE_STRING_FILE_CREATE) {
f = fopen(fn, "we");
- if (!f)
- return -errno;
+ if (!f) {
+ r = -errno;
+ goto fail;
+ }
} else {
int fd;
/* We manually build our own version of fopen(..., "we") that
* works without O_CREAT */
fd = open(fn, O_WRONLY|O_CLOEXEC|O_NOCTTY);
- if (fd < 0)
- return -errno;
+ if (fd < 0) {
+ r = -errno;
+ goto fail;
+ }
f = fdopen(fd, "we");
if (!f) {
+ r = -errno;
safe_close(fd);
- return -errno;
+ goto fail;
}
}
- return write_string_stream(f, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE));
+ r = write_string_stream(f, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE));
+ if (r < 0)
+ goto fail;
+
+ return 0;
+
+fail:
+ if (!(flags & WRITE_STRING_FILE_VERIFY_ON_FAILURE))
+ return r;
+
+ f = safe_fclose(f);
+
+ /* OK, the operation failed, but let's see if the right
+ * contents in place already. If so, eat up the error. */
+
+ q = verify_file(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE));
+ if (q <= 0)
+ return r;
+
+ return 0;
}
int read_one_line_file(const char *fn, char **line) {
@@ -139,15 +168,41 @@ int read_one_line_file(const char *fn, char **line) {
return 0;
}
-int verify_one_line_file(const char *fn, const char *line) {
- _cleanup_free_ char *value = NULL;
- int r;
+int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {
+ _cleanup_fclose_ FILE *f = NULL;
+ _cleanup_free_ char *buf = NULL;
+ size_t l, k;
- r = read_one_line_file(fn, &value);
- if (r < 0)
- return r;
+ assert(fn);
+ assert(blob);
+
+ l = strlen(blob);
+
+ if (accept_extra_nl && endswith(blob, "\n"))
+ accept_extra_nl = false;
+
+ buf = malloc(l + accept_extra_nl + 1);
+ if (!buf)
+ return -ENOMEM;
- return streq(value, line);
+ f = fopen(fn, "re");
+ if (!f)
+ return -errno;
+
+ /* We try to read one byte more than we need, so that we know whether we hit eof */
+ errno = 0;
+ k = fread(buf, 1, l + accept_extra_nl + 1, f);
+ if (ferror(f))
+ return errno > 0 ? -errno : -EIO;
+
+ if (k != l && k != l + accept_extra_nl)
+ return 0;
+ if (memcmp(buf, blob, l) != 0)
+ return 0;
+ if (k > l && buf[l] != '\n')
+ return 0;
+
+ return 1;
}
int read_full_stream(FILE *f, char **contents, size_t *size) {
diff --git a/src/basic/fileio.h b/src/basic/fileio.h
index 5f2c941498..95e8698941 100644
--- a/src/basic/fileio.h
+++ b/src/basic/fileio.h
@@ -34,6 +34,7 @@ typedef enum {
WRITE_STRING_FILE_CREATE = 1,
WRITE_STRING_FILE_ATOMIC = 2,
WRITE_STRING_FILE_AVOID_NEWLINE = 4,
+ WRITE_STRING_FILE_VERIFY_ON_FAILURE = 8,
} WriteStringFileFlags;
int write_string_stream(FILE *f, const char *line, bool enforce_newline);
@@ -43,7 +44,7 @@ int read_one_line_file(const char *fn, char **line);
int read_full_file(const char *fn, char **contents, size_t *size);
int read_full_stream(FILE *f, char **contents, size_t *size);
-int verify_one_line_file(const char *fn, const char *line);
+int verify_file(const char *fn, const char *blob, bool accept_extra_nl);
int parse_env_file(const char *fname, const char *separator, ...) _sentinel_;
int load_env_file(FILE *f, const char *fname, const char *separator, char ***l);
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 00c57b2960..07910c2c3b 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -1859,14 +1859,9 @@ static int link_set_ipv4_forward(Link *link) {
p = strjoina("/proc/sys/net/ipv4/conf/", link->ifname, "/forwarding");
v = one_zero(link_ipv4_forward_enabled(link));
- r = write_string_file(p, v, 0);
- if (r < 0) {
- /* If the right value is set anyway, don't complain */
- if (verify_one_line_file(p, v) > 0)
- return 0;
-
+ r = write_string_file(p, v, WRITE_STRING_FILE_VERIFY_ON_FAILURE);
+ if (r < 0)
log_link_warning_errno(link, r, "Cannot configure IPv4 forwarding for interface %s: %m", link->ifname);
- }
return 0;
}
@@ -1888,14 +1883,9 @@ static int link_set_ipv6_forward(Link *link) {
p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/forwarding");
v = one_zero(link_ipv6_forward_enabled(link));
- r = write_string_file(p, v, 0);
- if (r < 0) {
- /* If the right value is set anyway, don't complain */
- if (verify_one_line_file(p, v) > 0)
- return 0;
-
+ r = write_string_file(p, v, WRITE_STRING_FILE_VERIFY_ON_FAILURE);
+ if (r < 0)
log_link_warning_errno(link, r, "Cannot configure IPv6 forwarding for interface: %m");
- }
return 0;
}
@@ -1917,14 +1907,9 @@ static int link_set_ipv6_privacy_extensions(Link *link) {
p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/use_tempaddr");
xsprintf(buf, "%u", link->network->ipv6_privacy_extensions);
- r = write_string_file(p, buf, 0);
- if (r < 0) {
- /* If the right value is set anyway, don't complain */
- if (verify_one_line_file(p, buf) > 0)
- return 0;
-
+ r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE);
+ if (r < 0)
log_link_warning_errno(link, r, "Cannot configure IPv6 privacy extension for interface: %m");
- }
return 0;
}
@@ -1943,14 +1928,9 @@ static int link_set_ipv6_accept_ra(Link *link) {
p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/accept_ra");
/* we handle router advertisments ourselves, tell the kernel to GTFO */
- r = write_string_file(p, "0", 0);
- if (r < 0) {
- /* If the right value is set anyway, don't complain */
- if (verify_one_line_file(p, "0") > 0)
- return 0;
-
+ r = write_string_file(p, "0", WRITE_STRING_FILE_VERIFY_ON_FAILURE);
+ if (r < 0)
log_link_warning_errno(link, r, "Cannot disable kernel IPv6 accept_ra for interface: %m");
- }
return 0;
}
@@ -1974,14 +1954,9 @@ static int link_set_ipv6_dad_transmits(Link *link) {
xsprintf(buf, "%u", link->network->ipv6_dad_transmits);
- r = write_string_file(p, buf, 0);
- if (r < 0) {
- /* If the right value is set anyway, don't complain */
- if (verify_one_line_file(p, buf) > 0)
- return 0;
-
+ r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE);
+ if (r < 0)
log_link_warning_errno(link, r, "Cannot set IPv6 dad transmits for interface: %m");
- }
return 0;
}
@@ -2005,14 +1980,9 @@ static int link_set_ipv6_hop_limit(Link *link) {
xsprintf(buf, "%u", link->network->ipv6_hop_limit);
- r = write_string_file(p, buf, 0);
- if (r < 0) {
- /* If the right value is set anyway, don't complain */
- if (verify_one_line_file(p, buf) > 0)
- return 0;
-
+ r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE);
+ if (r < 0)
log_link_warning_errno(link, r, "Cannot set IPv6 hop limit for interface: %m");
- }
return 0;
}
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index e588681b86..bde3c7c3cf 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -363,6 +363,26 @@ static void test_write_string_file_no_create(void) {
unlink(fn);
}
+static void test_write_string_file_verify(void) {
+ _cleanup_free_ char *buf = NULL, *buf2 = NULL;
+ int r;
+
+ assert_se(read_one_line_file("/proc/cmdline", &buf) >= 0);
+ assert_se((buf2 = strjoin(buf, "\n", NULL)));
+
+ r = write_string_file("/proc/cmdline", buf, 0);
+ assert_se(r == -EACCES || r == -EIO);
+ r = write_string_file("/proc/cmdline", buf2, 0);
+ assert_se(r == -EACCES || r == -EIO);
+
+ assert_se(write_string_file("/proc/cmdline", buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE) == 0);
+ assert_se(write_string_file("/proc/cmdline", buf2, WRITE_STRING_FILE_VERIFY_ON_FAILURE) == 0);
+
+ r = write_string_file("/proc/cmdline", buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE|WRITE_STRING_FILE_AVOID_NEWLINE);
+ assert_se(r == -EACCES || r == -EIO);
+ assert_se(write_string_file("/proc/cmdline", buf2, WRITE_STRING_FILE_VERIFY_ON_FAILURE|WRITE_STRING_FILE_AVOID_NEWLINE) == 0);
+}
+
static void test_load_env_file_pairs(void) {
char fn[] = "/tmp/test-load_env_file_pairs-XXXXXX";
int fd;
@@ -419,6 +439,7 @@ int main(int argc, char *argv[]) {
test_write_string_stream();
test_write_string_file();
test_write_string_file_no_create();
+ test_write_string_file_verify();
test_load_env_file_pairs();
return 0;