Merge pull request #20676 from gogsbread/sysctl-minimize-sideeffect

sysctl: minimize side effects when running `systemd-sysctl`
This commit is contained in:
Lennart Poettering 2021-09-29 09:17:48 +02:00 committed by GitHub
commit 41a978fdb1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 103 additions and 22 deletions

View file

@ -148,6 +148,30 @@ int write_string_stream_ts(
return -EBADF;
}
if (flags & WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL) {
_cleanup_free_ char *t = NULL;
/* If value to be written is same as that of the existing value, then suppress the write. */
if (fd < 0) {
fd = fileno(f);
if (fd < 0)
return -EBADF;
}
/* Read an additional byte to detect cases where the prefix matches but the rest
* doesn't. Also, 0 returned by read_virtual_file_fd() means the read was truncated and
* it won't be equal to the new value. */
if (read_virtual_file_fd(fd, strlen(line)+1, &t, NULL) > 0 &&
streq_skip_trailing_chars(line, t, NEWLINE)) {
log_debug("No change in value '%s', supressing write", line);
return 0;
}
if (lseek(fd, 0, SEEK_SET) < 0)
return -errno;
}
needs_nl = !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n");
if (needs_nl && (flags & WRITE_STRING_FILE_DISABLE_BUFFER)) {
@ -263,10 +287,11 @@ int write_string_file_ts(
assert(!ts);
/* We manually build our own version of fopen(..., "we") that works without O_CREAT and with O_NOFOLLOW if needed. */
fd = open(fn, O_WRONLY|O_CLOEXEC|O_NOCTTY |
fd = open(fn, O_CLOEXEC|O_NOCTTY |
(FLAGS_SET(flags, WRITE_STRING_FILE_NOFOLLOW) ? O_NOFOLLOW : 0) |
(FLAGS_SET(flags, WRITE_STRING_FILE_CREATE) ? O_CREAT : 0) |
(FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0),
(FLAGS_SET(flags, WRITE_STRING_FILE_TRUNCATE) ? O_TRUNC : 0) |
(FLAGS_SET(flags, WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL) ? O_RDWR : O_WRONLY),
(FLAGS_SET(flags, WRITE_STRING_FILE_MODE_0600) ? 0600 : 0666));
if (fd < 0) {
r = -errno;
@ -375,9 +400,8 @@ int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {
return 1;
}
int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size) {
int read_virtual_file_fd(int fd, size_t max_size, char **ret_contents, size_t *ret_size) {
_cleanup_free_ char *buf = NULL;
_cleanup_close_ int fd = -1;
size_t n, size;
int n_retries;
bool truncated = false;
@ -395,10 +419,7 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
* contents* may be returned. (Though the read is still done using one syscall.) Returns 0 on
* partial success, 1 if untruncated contents were read. */
fd = open(filename, O_RDONLY|O_NOCTTY|O_CLOEXEC);
if (fd < 0)
return -errno;
assert(fd >= 0);
assert(max_size <= READ_VIRTUAL_BYTES_MAX || max_size == SIZE_MAX);
/* Limit the number of attempts to read the number of bytes returned by fstat(). */
@ -434,8 +455,8 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
n_retries--;
} else if (n_retries > 1) {
/* Files in /proc are generally smaller than the page size so let's start with a page size
* buffer from malloc and only use the max buffer on the final try. */
/* Files in /proc are generally smaller than the page size so let's start with
* a page size buffer from malloc and only use the max buffer on the final try. */
size = MIN3(page_size() - 1, READ_VIRTUAL_BYTES_MAX, max_size);
n_retries = 1;
} else {
@ -524,6 +545,18 @@ int read_virtual_file(const char *filename, size_t max_size, char **ret_contents
return !truncated;
}
int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size) {
_cleanup_close_ int fd = -1;
assert(filename);
fd = open(filename, O_RDONLY | O_NOCTTY | O_CLOEXEC);
if (fd < 0)
return -errno;
return read_virtual_file_fd(fd, max_size, ret_contents, ret_size);
}
int read_full_stream_full(
FILE *f,
const char *filename,

View file

@ -15,17 +15,18 @@
#define LONG_LINE_MAX (1U*1024U*1024U)
typedef enum {
WRITE_STRING_FILE_CREATE = 1 << 0,
WRITE_STRING_FILE_TRUNCATE = 1 << 1,
WRITE_STRING_FILE_ATOMIC = 1 << 2,
WRITE_STRING_FILE_AVOID_NEWLINE = 1 << 3,
WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1 << 4,
WRITE_STRING_FILE_VERIFY_IGNORE_NEWLINE = 1 << 5,
WRITE_STRING_FILE_SYNC = 1 << 6,
WRITE_STRING_FILE_DISABLE_BUFFER = 1 << 7,
WRITE_STRING_FILE_NOFOLLOW = 1 << 8,
WRITE_STRING_FILE_MKDIR_0755 = 1 << 9,
WRITE_STRING_FILE_MODE_0600 = 1 << 10,
WRITE_STRING_FILE_CREATE = 1 << 0,
WRITE_STRING_FILE_TRUNCATE = 1 << 1,
WRITE_STRING_FILE_ATOMIC = 1 << 2,
WRITE_STRING_FILE_AVOID_NEWLINE = 1 << 3,
WRITE_STRING_FILE_VERIFY_ON_FAILURE = 1 << 4,
WRITE_STRING_FILE_VERIFY_IGNORE_NEWLINE = 1 << 5,
WRITE_STRING_FILE_SYNC = 1 << 6,
WRITE_STRING_FILE_DISABLE_BUFFER = 1 << 7,
WRITE_STRING_FILE_NOFOLLOW = 1 << 8,
WRITE_STRING_FILE_MKDIR_0755 = 1 << 9,
WRITE_STRING_FILE_MODE_0600 = 1 << 10,
WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL = 1 << 11,
/* And before you wonder, why write_string_file_atomic_label_ts() is a separate function instead of just one
more flag here: it's about linking: we don't want to pull -lselinux into all users of write_string_file()
@ -67,6 +68,7 @@ static inline int read_full_file(const char *filename, char **ret_contents, size
return read_full_file_full(AT_FDCWD, filename, UINT64_MAX, SIZE_MAX, 0, NULL, ret_contents, ret_size);
}
int read_virtual_file_fd(int fd, size_t max_size, char **ret_contents, size_t *ret_size);
int read_virtual_file(const char *filename, size_t max_size, char **ret_contents, size_t *ret_size);
static inline int read_full_virtual_file(const char *filename, char **ret_contents, size_t *ret_size) {
return read_virtual_file(filename, SIZE_MAX, ret_contents, ret_size);

View file

@ -1146,3 +1146,19 @@ int string_contains_word_strv(const char *string, const char *separators, char *
*ret_word = found;
return !!found;
}
bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok) {
if (!s1 && !s2)
return true;
if (!s1 || !s2)
return false;
if (!ok)
ok = WHITESPACE;
for (; *s1 && *s2; s1++, s2++)
if (*s1 != *s2)
break;
return in_charset(s1, ok) && in_charset(s2, ok);
}

View file

@ -226,3 +226,5 @@ int string_contains_word_strv(const char *string, const char *separators, char *
static inline int string_contains_word(const char *string, const char *separators, const char *word) {
return string_contains_word_strv(string, separators, STRV_MAKE(word), NULL);
}
bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok);

View file

@ -58,7 +58,7 @@ int sysctl_write(const char *property, const char *value) {
log_debug("Setting '%s' to '%s'", p, value);
return write_string_file(p, value, WRITE_STRING_FILE_VERIFY_ON_FAILURE | WRITE_STRING_FILE_DISABLE_BUFFER);
return write_string_file(p, value, WRITE_STRING_FILE_VERIFY_ON_FAILURE | WRITE_STRING_FILE_DISABLE_BUFFER | WRITE_STRING_FILE_SUPPRESS_REDUNDANT_VIRTUAL);
}
int sysctl_writef(const char *property, const char *format, ...) {

View file

@ -1000,6 +1000,33 @@ static void test_strextendf(void) {
assert_se(streq(p, "<77>,<99>,< 88>,<00001234>"));
}
static void test_streq_skip_trailing_chars(void) {
log_info("/* %s */", __func__);
/* NULL is WHITESPACE by default*/
assert_se(streq_skip_trailing_chars("foo bar", "foo bar", NULL));
assert_se(streq_skip_trailing_chars("foo", "foo", NULL));
assert_se(streq_skip_trailing_chars("foo bar ", "foo bar", NULL));
assert_se(streq_skip_trailing_chars("foo bar", "foo bar\t\t", NULL));
assert_se(streq_skip_trailing_chars("foo bar ", "foo bar\t\t", NULL));
assert_se(streq_skip_trailing_chars("foo\nbar", "foo\nbar", NULL));
assert_se(streq_skip_trailing_chars("\t\tfoo bar", "\t\tfoo bar", NULL));
assert_se(streq_skip_trailing_chars(" foo bar\t", " foo bar\n", NULL));
assert_se(!streq_skip_trailing_chars("foobar", "foo bar", NULL));
assert_se(!streq_skip_trailing_chars("foo\nbar", "foo\tbar", NULL));
assert_se(!streq_skip_trailing_chars("\t\nfoo bar", "\t foo bar", NULL));
assert_se(streq_skip_trailing_chars("foo bar ", "foo bar", WHITESPACE));
assert_se(!streq_skip_trailing_chars("foo bar ", "foo bar", NEWLINE));
assert_se(streq_skip_trailing_chars(NULL, NULL, NULL));
assert_se(streq_skip_trailing_chars("", "", NULL));
assert_se(!streq_skip_trailing_chars(NULL, "foo bar", NULL));
assert_se(!streq_skip_trailing_chars("foo", NULL, NULL));
assert_se(!streq_skip_trailing_chars("", "f", NULL));
}
int main(int argc, char *argv[]) {
test_setup_logging(LOG_DEBUG);
@ -1039,6 +1066,7 @@ int main(int argc, char *argv[]) {
test_string_contains_word();
test_strverscmp_improved();
test_strextendf();
test_streq_skip_trailing_chars();
return 0;
}