diff options
-rw-r--r-- | src/basic/calendarspec.c | 6 | ||||
-rw-r--r-- | src/basic/time-util.c | 13 | ||||
-rw-r--r-- | src/journal-remote/journal-remote-write.c | 2 | ||||
-rw-r--r-- | src/journal-remote/journal-remote.c | 2 | ||||
-rw-r--r-- | src/journal/journal-file.c | 259 | ||||
-rw-r--r-- | src/journal/journal-file.h | 21 | ||||
-rw-r--r-- | src/journal/journald-server.c | 38 | ||||
-rw-r--r-- | src/journal/journald-server.h | 2 | ||||
-rw-r--r-- | src/journal/sd-journal.c | 8 | ||||
-rw-r--r-- | src/journal/test-journal-flush.c | 4 | ||||
-rw-r--r-- | src/journal/test-journal-interleaving.c | 10 | ||||
-rw-r--r-- | src/journal/test-journal-stream.c | 12 | ||||
-rw-r--r-- | src/journal/test-journal-verify.c | 12 | ||||
-rw-r--r-- | src/journal/test-journal.c | 24 | ||||
-rw-r--r-- | src/libsystemd/sd-device/sd-device.c | 12 | ||||
-rw-r--r-- | src/test/test-siphash24.c | 47 | ||||
-rw-r--r-- | src/udev/udev-rules.c | 622 | ||||
-rw-r--r-- | src/udev/udev.h | 6 |
18 files changed, 624 insertions, 476 deletions
diff --git a/src/basic/calendarspec.c b/src/basic/calendarspec.c index 775879076d..00fc39a499 100644 --- a/src/basic/calendarspec.c +++ b/src/basic/calendarspec.c @@ -978,7 +978,7 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) { for (;;) { /* Normalize the current date */ - mktime_or_timegm(&c, spec->utc); + (void) mktime_or_timegm(&c, spec->utc); c.tm_isdst = -1; c.tm_year += 1900; @@ -990,8 +990,10 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) { c.tm_mday = 1; c.tm_hour = c.tm_min = c.tm_sec = tm_usec = 0; } - if (r < 0 || tm_out_of_bounds(&c, spec->utc)) + if (r < 0) return r; + if (tm_out_of_bounds(&c, spec->utc)) + return -ENOENT; c.tm_mon += 1; r = find_matching_component(spec->month, &c.tm_mon); diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 130acaa9de..9bfd8f4f7a 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -139,8 +139,7 @@ dual_timestamp* dual_timestamp_from_boottime_or_monotonic(dual_timestamp *ts, us usec_t timespec_load(const struct timespec *ts) { assert(ts); - if (ts->tv_sec == (time_t) -1 && - ts->tv_nsec == (long) -1) + if (ts->tv_sec == (time_t) -1 && ts->tv_nsec == (long) -1) return USEC_INFINITY; if ((usec_t) ts->tv_sec > (UINT64_MAX - (ts->tv_nsec / NSEC_PER_USEC)) / USEC_PER_SEC) @@ -154,13 +153,13 @@ usec_t timespec_load(const struct timespec *ts) { static nsec_t timespec_load_nsec(const struct timespec *ts) { assert(ts); - if (ts->tv_sec == (time_t) -1 && - ts->tv_nsec == (long) -1) + if (ts->tv_sec == (time_t) -1 && ts->tv_nsec == (long) -1) return NSEC_INFINITY; - return - (nsec_t) ts->tv_sec * NSEC_PER_SEC + - (nsec_t) ts->tv_nsec; + if ((nsec_t) ts->tv_sec >= (UINT64_MAX - ts->tv_nsec) / NSEC_PER_SEC) + return NSEC_INFINITY; + + return (nsec_t) ts->tv_sec * NSEC_PER_SEC + (nsec_t) ts->tv_nsec; } struct timespec *timespec_store(struct timespec *ts, usec_t u) { diff --git a/src/journal-remote/journal-remote-write.c b/src/journal-remote/journal-remote-write.c index 5fab74e5cc..7bba52566e 100644 --- a/src/journal-remote/journal-remote-write.c +++ b/src/journal-remote/journal-remote-write.c @@ -54,7 +54,7 @@ void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new) { **********************************************************************/ static int do_rotate(JournalFile **f, bool compress, bool seal) { - int r = journal_file_rotate(f, compress, seal); + int r = journal_file_rotate(f, compress, seal, NULL); if (r < 0) { if (*f) log_error_errno(r, "Failed to rotate %s: %m", (*f)->path); diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 3ce6fe27b3..44f9a9b44f 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -203,7 +203,7 @@ static int open_output(Writer *w, const char* host) { O_RDWR|O_CREAT, 0640, arg_compress, arg_seal, &w->metrics, - w->mmap, + w->mmap, NULL, NULL, &w->journal); if (r < 0) log_error_errno(r, "Failed to open output journal %s: %m", diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 994d1ec5d8..f5e2952c99 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -20,6 +20,7 @@ #include <errno.h> #include <fcntl.h> #include <linux/fs.h> +#include <pthread.h> #include <stddef.h> #include <sys/mman.h> #include <sys/statvfs.h> @@ -38,6 +39,7 @@ #include "parse-util.h" #include "random-util.h" #include "sd-event.h" +#include "set.h" #include "string-util.h" #include "xattr-util.h" @@ -86,33 +88,127 @@ /* The mmap context to use for the header we pick as one above the last defined typed */ #define CONTEXT_HEADER _OBJECT_TYPE_MAX -static int journal_file_set_online(JournalFile *f) { +/* This may be called from a separate thread to prevent blocking the caller for the duration of fsync(). + * As a result we use atomic operations on f->offline_state for inter-thread communications with + * journal_file_set_offline() and journal_file_set_online(). */ +static void journal_file_set_offline_internal(JournalFile *f) { assert(f); + assert(f->fd >= 0); + assert(f->header); - if (!f->writable) - return -EPERM; + for (;;) { + switch (f->offline_state) { + case OFFLINE_CANCEL: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_CANCEL, OFFLINE_DONE)) + continue; + return; - if (!(f->fd >= 0 && f->header)) - return -EINVAL; + case OFFLINE_AGAIN_FROM_SYNCING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_AGAIN_FROM_SYNCING, OFFLINE_SYNCING)) + continue; + break; + + case OFFLINE_AGAIN_FROM_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_AGAIN_FROM_OFFLINING, OFFLINE_SYNCING)) + continue; + break; + + case OFFLINE_SYNCING: + (void) fsync(f->fd); + + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_SYNCING, OFFLINE_OFFLINING)) + continue; + + f->header->state = STATE_OFFLINE; + (void) fsync(f->fd); + break; + + case OFFLINE_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_OFFLINING, OFFLINE_DONE)) + continue; + /* fall through */ + + case OFFLINE_DONE: + return; + + case OFFLINE_JOINED: + log_debug("OFFLINE_JOINED unexpected offline state for journal_file_set_offline_internal()"); + return; + } + } +} + +static void * journal_file_set_offline_thread(void *arg) { + JournalFile *f = arg; + + journal_file_set_offline_internal(f); + + return NULL; +} + +static int journal_file_set_offline_thread_join(JournalFile *f) { + int r; + + assert(f); + + if (f->offline_state == OFFLINE_JOINED) + return 0; + + r = pthread_join(f->offline_thread, NULL); + if (r) + return -r; + + f->offline_state = OFFLINE_JOINED; if (mmap_cache_got_sigbus(f->mmap, f->fd)) return -EIO; - switch (f->header->state) { - case STATE_ONLINE: - return 0; + return 0; +} - case STATE_OFFLINE: - f->header->state = STATE_ONLINE; - fsync(f->fd); - return 0; +/* Trigger a restart if the offline thread is mid-flight in a restartable state. */ +static bool journal_file_set_offline_try_restart(JournalFile *f) { + for (;;) { + switch (f->offline_state) { + case OFFLINE_AGAIN_FROM_SYNCING: + case OFFLINE_AGAIN_FROM_OFFLINING: + return true; + + case OFFLINE_CANCEL: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_CANCEL, OFFLINE_AGAIN_FROM_SYNCING)) + continue; + return true; + + case OFFLINE_SYNCING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_SYNCING, OFFLINE_AGAIN_FROM_SYNCING)) + continue; + return true; + + case OFFLINE_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_OFFLINING, OFFLINE_AGAIN_FROM_OFFLINING)) + continue; + return true; default: - return -EINVAL; + return false; + } } } -int journal_file_set_offline(JournalFile *f) { +/* Sets a journal offline. + * + * If wait is false then an offline is dispatched in a separate thread for a + * subsequent journal_file_set_offline() or journal_file_set_online() of the + * same journal to synchronize with. + * + * If wait is true, then either an existing offline thread will be restarted + * and joined, or if none exists the offline is simply performed in this + * context without involving another thread. + */ +int journal_file_set_offline(JournalFile *f, bool wait) { + bool restarted; + int r; + assert(f); if (!f->writable) @@ -124,19 +220,107 @@ int journal_file_set_offline(JournalFile *f) { if (f->header->state != STATE_ONLINE) return 0; - fsync(f->fd); + /* Restart an in-flight offline thread and wait if needed, or join a lingering done one. */ + restarted = journal_file_set_offline_try_restart(f); + if ((restarted && wait) || !restarted) { + r = journal_file_set_offline_thread_join(f); + if (r < 0) + return r; + } + + if (restarted) + return 0; + + /* Initiate a new offline. */ + f->offline_state = OFFLINE_SYNCING; - if (mmap_cache_got_sigbus(f->mmap, f->fd)) - return -EIO; + if (wait) /* Without using a thread if waiting. */ + journal_file_set_offline_internal(f); + else { + r = pthread_create(&f->offline_thread, NULL, journal_file_set_offline_thread, f); + if (r > 0) + return -r; + } + + return 0; +} + +static int journal_file_set_online(JournalFile *f) { + bool joined = false; - f->header->state = STATE_OFFLINE; + assert(f); + + if (!f->writable) + return -EPERM; + + if (!(f->fd >= 0 && f->header)) + return -EINVAL; + + while (!joined) { + switch (f->offline_state) { + case OFFLINE_JOINED: + /* No offline thread, no need to wait. */ + joined = true; + break; + + case OFFLINE_SYNCING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_SYNCING, OFFLINE_CANCEL)) + continue; + /* Canceled syncing prior to offlining, no need to wait. */ + break; + + case OFFLINE_AGAIN_FROM_SYNCING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_AGAIN_FROM_SYNCING, OFFLINE_CANCEL)) + continue; + /* Canceled restart from syncing, no need to wait. */ + break; + + case OFFLINE_AGAIN_FROM_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->offline_state, OFFLINE_AGAIN_FROM_OFFLINING, OFFLINE_CANCEL)) + continue; + /* Canceled restart from offlining, must wait for offlining to complete however. */ + + /* fall through to wait */ + default: { + int r; + + r = journal_file_set_offline_thread_join(f); + if (r < 0) + return r; + + joined = true; + break; + } + } + } if (mmap_cache_got_sigbus(f->mmap, f->fd)) return -EIO; - fsync(f->fd); + switch (f->header->state) { + case STATE_ONLINE: + return 0; - return 0; + case STATE_OFFLINE: + f->header->state = STATE_ONLINE; + (void) fsync(f->fd); + return 0; + + default: + return -EINVAL; + } +} + +bool journal_file_is_offlining(JournalFile *f) { + assert(f); + + __sync_synchronize(); + + if (f->offline_state == OFFLINE_DONE || + f->offline_state == OFFLINE_JOINED) + return false; + + return true; } JournalFile* journal_file_close(JournalFile *f) { @@ -159,7 +343,7 @@ JournalFile* journal_file_close(JournalFile *f) { sd_event_source_unref(f->post_change_timer); } - journal_file_set_offline(f); + journal_file_set_offline(f, true); if (f->mmap && f->fd >= 0) mmap_cache_close_fd(f->mmap, f->fd); @@ -203,6 +387,15 @@ JournalFile* journal_file_close(JournalFile *f) { return NULL; } +void journal_file_close_set(Set *s) { + JournalFile *f; + + assert(s); + + while ((f = set_steal_first(s))) + (void) journal_file_close(f); +} + static int journal_file_init_header(JournalFile *f, JournalFile *template) { Header h = {}; ssize_t k; @@ -263,7 +456,7 @@ static int journal_file_refresh_header(JournalFile *f) { r = journal_file_set_online(f); /* Sync the online state to disk */ - fsync(f->fd); + (void) fsync(f->fd); return r; } @@ -2710,6 +2903,7 @@ int journal_file_open( bool seal, JournalMetrics *metrics, MMapCache *mmap_cache, + Set *deferred_closes, JournalFile *template, JournalFile **ret) { @@ -2829,6 +3023,9 @@ int journal_file_open( f->header = h; if (!newly_created) { + if (deferred_closes) + journal_file_close_set(deferred_closes); + r = journal_file_verify_header(f); if (r < 0) goto fail; @@ -2898,12 +3095,12 @@ fail: if (f->fd >= 0 && mmap_cache_got_sigbus(f->mmap, f->fd)) r = -EIO; - journal_file_close(f); + (void) journal_file_close(f); return r; } -int journal_file_rotate(JournalFile **f, bool compress, bool seal) { +int journal_file_rotate(JournalFile **f, bool compress, bool seal, Set *deferred_closes) { _cleanup_free_ char *p = NULL; size_t l; JournalFile *old_file, *new_file = NULL; @@ -2943,8 +3140,13 @@ int journal_file_rotate(JournalFile **f, bool compress, bool seal) { * we archive them */ old_file->defrag_on_close = true; - r = journal_file_open(old_file->path, old_file->flags, old_file->mode, compress, seal, NULL, old_file->mmap, old_file, &new_file); - journal_file_close(old_file); + r = journal_file_open(old_file->path, old_file->flags, old_file->mode, compress, seal, NULL, old_file->mmap, deferred_closes, old_file, &new_file); + + if (deferred_closes && + set_put(deferred_closes, old_file) >= 0) + (void) journal_file_set_offline(old_file, false); + else + (void) journal_file_close(old_file); *f = new_file; return r; @@ -2958,6 +3160,7 @@ int journal_file_open_reliably( bool seal, JournalMetrics *metrics, MMapCache *mmap_cache, + Set *deferred_closes, JournalFile *template, JournalFile **ret) { @@ -2965,7 +3168,7 @@ int journal_file_open_reliably( size_t l; _cleanup_free_ char *p = NULL; - r = journal_file_open(fname, flags, mode, compress, seal, metrics, mmap_cache, template, ret); + r = journal_file_open(fname, flags, mode, compress, seal, metrics, mmap_cache, deferred_closes, template, ret); if (!IN_SET(r, -EBADMSG, /* corrupted */ -ENODATA, /* truncated */ @@ -3006,7 +3209,7 @@ int journal_file_open_reliably( log_warning_errno(r, "File %s corrupted or uncleanly shut down, renaming and replacing.", fname); - return journal_file_open(fname, flags, mode, compress, seal, metrics, mmap_cache, template, ret); + return journal_file_open(fname, flags, mode, compress, seal, metrics, mmap_cache, deferred_closes, template, ret); } int journal_file_copy_entry(JournalFile *from, JournalFile *to, Object *o, uint64_t p, uint64_t *seqnum, Object **ret, uint64_t *offset) { diff --git a/src/journal/journal-file.h b/src/journal/journal-file.h index 07b9561b8a..9ad6013359 100644 --- a/src/journal/journal-file.h +++ b/src/journal/journal-file.h @@ -63,6 +63,16 @@ typedef enum LocationType { LOCATION_SEEK } LocationType; +typedef enum OfflineState { + OFFLINE_JOINED, + OFFLINE_SYNCING, + OFFLINE_OFFLINING, + OFFLINE_CANCEL, + OFFLINE_AGAIN_FROM_SYNCING, + OFFLINE_AGAIN_FROM_OFFLINING, + OFFLINE_DONE +} OfflineState; + typedef struct JournalFile { int fd; @@ -105,6 +115,9 @@ typedef struct JournalFile { OrderedHashmap *chain_cache; + pthread_t offline_thread; + volatile OfflineState offline_state; + #if defined(HAVE_XZ) || defined(HAVE_LZ4) void *compress_buffer; size_t compress_buffer_size; @@ -136,11 +149,14 @@ int journal_file_open( bool seal, JournalMetrics *metrics, MMapCache *mmap_cache, + Set *deferred_closes, JournalFile *template, JournalFile **ret); -int journal_file_set_offline(JournalFile *f); +int journal_file_set_offline(JournalFile *f, bool wait); +bool journal_file_is_offlining(JournalFile *f); JournalFile* journal_file_close(JournalFile *j); +void journal_file_close_set(Set *s); int journal_file_open_reliably( const char *fname, @@ -150,6 +166,7 @@ int journal_file_open_reliably( bool seal, JournalMetrics *metrics, MMapCache *mmap_cache, + Set *deferred_closes, JournalFile *template, JournalFile **ret); @@ -223,7 +240,7 @@ int journal_file_copy_entry(JournalFile *from, JournalFile *to, Object *o, uint6 void journal_file_dump(JournalFile *f); void journal_file_print_header(JournalFile *f); -int journal_file_rotate(JournalFile **f, bool compress, bool seal); +int journal_file_rotate(JournalFile **f, bool compress, bool seal, Set *deferred_closes); void journal_file_post_change(JournalFile *f); int journal_file_enable_post_change_timer(JournalFile *f, sd_event *e, usec_t t); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index ee2db8d29f..d5937bd013 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -251,15 +251,15 @@ static int open_journal( assert(ret); if (reliably) - r = journal_file_open_reliably(fname, flags, 0640, s->compress, seal, metrics, s->mmap, NULL, &f); + r = journal_file_open_reliably(fname, flags, 0640, s->compress, seal, metrics, s->mmap, s->deferred_closes, NULL, &f); else - r = journal_file_open(fname, flags, 0640, s->compress, seal, metrics, s->mmap, NULL, &f); + r = journal_file_open(fname, flags, 0640, s->compress, seal, metrics, s->mmap, s->deferred_closes, NULL, &f); if (r < 0) return r; r = journal_file_enable_post_change_timer(f, s->event, POST_CHANGE_TIMER_INTERVAL_USEC); if (r < 0) { - journal_file_close(f); + (void) journal_file_close(f); return r; } @@ -302,7 +302,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) { /* Too many open? Then let's close one */ f = ordered_hashmap_steal_first(s->user_journals); assert(f); - journal_file_close(f); + (void) journal_file_close(f); } r = open_journal(s, true, p, O_RDWR|O_CREAT, s->seal, &s->system_metrics, &f); @@ -313,7 +313,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) { r = ordered_hashmap_put(s->user_journals, UID_TO_PTR(uid), f); if (r < 0) { - journal_file_close(f); + (void) journal_file_close(f); return s->system_journal; } @@ -333,7 +333,7 @@ static int do_rotate( if (!*f) return -EINVAL; - r = journal_file_rotate(f, s->compress, seal); + r = journal_file_rotate(f, s->compress, seal, s->deferred_closes); if (r < 0) if (*f) log_error_errno(r, "Failed to rotate %s: %m", (*f)->path); @@ -364,6 +364,13 @@ void server_rotate(Server *s) { /* Old file has been closed and deallocated */ ordered_hashmap_remove(s->user_journals, k); } + + /* Perform any deferred closes which aren't still offlining. */ + SET_FOREACH(f, s->deferred_closes, i) + if (!journal_file_is_offlining(f)) { + (void) set_remove(s->deferred_closes, f); + (void) journal_file_close(f); + } } void server_sync(Server *s) { @@ -372,13 +379,13 @@ void server_sync(Server *s) { int r; if (s->system_journal) { - r = journal_file_set_offline(s->system_journal); + r = journal_file_set_offline(s->system_journal, false); if (r < 0) log_warning_errno(r, "Failed to sync system journal, ignoring: %m"); } ORDERED_HASHMAP_FOREACH(f, s->user_journals, i) { - r = journal_file_set_offline(f); + r = journal_file_set_offline(f, false); if (r < 0) log_warning_errno(r, "Failed to sync user journal, ignoring: %m"); } @@ -1765,6 +1772,10 @@ int server_init(Server *s) { if (!s->mmap) return log_oom(); + s->deferred_closes = set_new(NULL); + if (!s->deferred_closes) + return log_oom(); + r = sd_event_default(&s->event); if (r < 0) return log_error_errno(r, "Failed to create event loop: %m"); @@ -1918,17 +1929,22 @@ void server_done(Server *s) { JournalFile *f; assert(s); + if (s->deferred_closes) { + journal_file_close_set(s->deferred_closes); + set_free(s->deferred_closes); + } + while (s->stdout_streams) stdout_stream_free(s->stdout_streams); if (s->system_journal) - journal_file_close(s->system_journal); + (void) journal_file_close(s->system_journal); if (s->runtime_journal) - journal_file_close(s->runtime_journal); + (void) journal_file_close(s->runtime_journal); while ((f = ordered_hashmap_steal_first(s->user_journals))) - journal_file_close(f); + (void) journal_file_close(f); ordered_hashmap_free(s->user_journals); diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h index b9551dda1b..e025a4cf90 100644 --- a/src/journal/journald-server.h +++ b/src/journal/journald-server.h @@ -130,6 +130,8 @@ struct Server { MMapCache *mmap; + Set *deferred_closes; + struct udev *udev; uint64_t *kernel_seqnum; diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 5a2a28a8d4..6ff1c67f5f 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -1248,7 +1248,7 @@ static int add_any_file(sd_journal *j, const char *path) { goto fail; } - r = journal_file_open(path, O_RDONLY, 0, false, false, NULL, j->mmap, NULL, &f); + r = journal_file_open(path, O_RDONLY, 0, false, false, NULL, j->mmap, NULL, NULL, &f); if (r < 0) { log_debug_errno(r, "Failed to open journal file %s: %m", path); goto fail; @@ -1258,7 +1258,7 @@ static int add_any_file(sd_journal *j, const char *path) { r = ordered_hashmap_put(j->files, f->path, f); if (r < 0) { - journal_file_close(f); + (void) journal_file_close(f); goto fail; } @@ -1343,7 +1343,7 @@ static void remove_file_real(sd_journal *j, JournalFile *f) { j->fields_file_lost = true; } - journal_file_close(f); + (void) journal_file_close(f); j->current_invalidate_counter ++; } @@ -1784,7 +1784,7 @@ _public_ void sd_journal_close(sd_journal *j) { sd_journal_flush_matches(j); while ((f = ordered_hashmap_steal_first(j->files))) - journal_file_close(f); + (void) journal_file_close(f); ordered_hashmap_free(j->files); diff --git a/src/journal/test-journal-flush.c b/src/journal/test-journal-flush.c index 7bd9c40366..93dc0e0d81 100644 --- a/src/journal/test-journal-flush.c +++ b/src/journal/test-journal-flush.c @@ -38,7 +38,7 @@ int main(int argc, char *argv[]) { assert_se(mkdtemp(dn)); fn = strappend(dn, "/test.journal"); - r = journal_file_open(fn, O_CREAT|O_RDWR, 0644, false, false, NULL, NULL, NULL, &new_journal); + r = journal_file_open(fn, O_CREAT|O_RDWR, 0644, false, false, NULL, NULL, NULL, NULL, &new_journal); assert_se(r >= 0); r = sd_journal_open(&j, 0); @@ -66,7 +66,7 @@ int main(int argc, char *argv[]) { sd_journal_close(j); - journal_file_close(new_journal); + (void) journal_file_close(new_journal); unlink(fn); assert_se(rmdir(dn) == 0); diff --git a/src/journal/test-journal-interleaving.c b/src/journal/test-journal-interleaving.c index 7f94990888..f887f43f0d 100644 --- a/src/journal/test-journal-interleaving.c +++ b/src/journal/test-journal-interleaving.c @@ -52,12 +52,12 @@ noreturn static void log_assert_errno(const char *text, int eno, const char *fil static JournalFile *test_open(const char *name) { JournalFile *f; - assert_ret(journal_file_open(name, O_RDWR|O_CREAT, 0644, true, false, NULL, NULL, NULL, &f)); + assert_ret(journal_file_open(name, O_RDWR|O_CREAT, 0644, true, false, NULL, NULL, NULL, NULL, &f)); return f; } static void test_close(JournalFile *f) { - journal_file_close (f); + (void) journal_file_close (f); } static void append_number(JournalFile *f, int n, uint64_t *seqnum) { @@ -217,7 +217,7 @@ static void test_sequence_numbers(void) { assert_se(chdir(t) >= 0); assert_se(journal_file_open("one.journal", O_RDWR|O_CREAT, 0644, - true, false, NULL, NULL, NULL, &one) == 0); + true, false, NULL, NULL, NULL, NULL, &one) == 0); append_number(one, 1, &seqnum); printf("seqnum=%"PRIu64"\n", seqnum); @@ -234,7 +234,7 @@ static void test_sequence_numbers(void) { memcpy(&seqnum_id, &one->header->seqnum_id, sizeof(sd_id128_t)); assert_se(journal_file_open("two.journal", O_RDWR|O_CREAT, 0644, - true, false, NULL, NULL, one, &two) == 0); + true, false, NULL, NULL, NULL, one, &two) == 0); assert_se(two->header->state == STATE_ONLINE); assert_se(!sd_id128_equal(two->header->file_id, one->header->file_id)); @@ -265,7 +265,7 @@ static void test_sequence_numbers(void) { seqnum = 0; assert_se(journal_file_open("two.journal", O_RDWR, 0, - true, false, NULL, NULL, NULL, &two) == 0); + true, false, NULL, NULL, NULL, NULL, &two) == 0); assert_se(sd_id128_equal(two->header->seqnum_id, seqnum_id)); diff --git a/src/journal/test-journal-stream.c b/src/journal/test-journal-stream.c index 4e6f8c0f7b..839ea5a9a5 100644 --- a/src/journal/test-journal-stream.c +++ b/src/journal/test-journal-stream.c @@ -92,9 +92,9 @@ int main(int argc, char *argv[]) { assert_se(mkdtemp(t)); assert_se(chdir(t) >= 0); - assert_se(journal_file_open("one.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, &one) == 0); - assert_se(journal_file_open("two.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, &two) == 0); - assert_se(journal_file_open("three.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, &three) == 0); + assert_se(journal_file_open("one.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, NULL, &one) == 0); + assert_se(journal_file_open("two.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, NULL, &two) == 0); + assert_se(journal_file_open("three.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, NULL, &three) == 0); for (i = 0; i < N_ENTRIES; i++) { char *p, *q; @@ -133,9 +133,9 @@ int main(int argc, char *argv[]) { free(q); } - journal_file_close(one); - journal_file_close(two); - journal_file_close(three); + (void) journal_file_close(one); + (void) journal_file_close(two); + (void) journal_file_close(three); assert_se(sd_journal_open_directory(&j, t, 0) >= 0); diff --git a/src/journal/test-journal-verify.c b/src/journal/test-journal-verify.c index a26c624f41..6b4643cd25 100644 --- a/src/journal/test-journal-verify.c +++ b/src/journal/test-journal-verify.c @@ -55,12 +55,12 @@ static int raw_verify(const char *fn, const char *verification_key) { JournalFile *f; int r; - r = journal_file_open(fn, O_RDONLY, 0666, true, !!verification_key, NULL, NULL, NULL, &f); + r = journal_file_open(fn, O_RDONLY, 0666, true, !!verification_key, NULL, NULL, NULL, NULL, &f); if (r < 0) return r; r = journal_file_verify(f, verification_key, NULL, NULL, NULL, false); - journal_file_close(f); + (void) journal_file_close(f); return r; } @@ -88,7 +88,7 @@ int main(int argc, char *argv[]) { log_info("Generating..."); - assert_se(journal_file_open("test.journal", O_RDWR|O_CREAT, 0666, true, !!verification_key, NULL, NULL, NULL, &f) == 0); + assert_se(journal_file_open("test.journal", O_RDWR|O_CREAT, 0666, true, !!verification_key, NULL, NULL, NULL, NULL, &f) == 0); for (n = 0; n < N_ENTRIES; n++) { struct iovec iovec; @@ -107,11 +107,11 @@ int main(int argc, char *argv[]) { free(test); } - journal_file_close(f); + (void) journal_file_close(f); log_info("Verifying..."); - assert_se(journal_file_open("test.journal", O_RDONLY, 0666, true, !!verification_key, NULL, NULL, NULL, &f) == 0); + assert_se(journal_file_open("test.journal", O_RDONLY, 0666, true, !!verification_key, NULL, NULL, NULL, NULL, &f) == 0); /* journal_file_print_header(f); */ journal_file_dump(f); @@ -123,7 +123,7 @@ int main(int argc, char *argv[]) { format_timestamp(b, sizeof(b), to), format_timespan(c, sizeof(c), total > to ? total - to : 0, 0)); - journal_file_close(f); + (void) journal_file_close(f); if (verification_key) { log_info("Toggling bits..."); diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index 0334b1cd1a..ea685af782 100644 --- a/src/journal/test-journal.c +++ b/src/journal/test-journal.c @@ -42,7 +42,7 @@ static void test_non_empty(void) { assert_se(mkdtemp(t)); assert_se(chdir(t) >= 0); - assert_se(journal_file_open("test.journal", O_RDWR|O_CREAT, 0666, true, true, NULL, NULL, NULL, &f) == 0); + assert_se(journal_file_open("test.journal", O_RDWR|O_CREAT, 0666, true, true, NULL, NULL, NULL, NULL, &f) == 0); dual_timestamp_get(&ts); @@ -104,10 +104,10 @@ static void test_non_empty(void) { assert_se(journal_file_move_to_entry_by_seqnum(f, 10, DIRECTION_DOWN, &o, NULL) == 0); - journal_file_rotate(&f, true, true); - journal_file_rotate(&f, true, true); + journal_file_rotate(&f, true, true, NULL); + journal_file_rotate(&f, true, true, NULL); - journal_file_close(f); + (void) journal_file_close(f); log_info("Done..."); @@ -131,13 +131,13 @@ static void test_empty(void) { assert_se(mkdtemp(t)); assert_se(chdir(t) >= 0); - assert_se(journal_file_open("test.journal", O_RDWR|O_CREAT, 0666, false, false, NULL, NULL, NULL, &f1) == 0); + assert_se(journal_file_open("test.journal", O_RDWR|O_CREAT, 0666, false, false, NULL, NULL, NULL, NULL, &f1) == 0); - assert_se(journal_file_open("test-compress.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, &f2) == 0); + assert_se(journal_file_open("test-compress.journal", O_RDWR|O_CREAT, 0666, true, false, NULL, NULL, NULL, NULL, &f2) == 0); - assert_se(journal_file_open("test-seal.journal", O_RDWR|O_CREAT, 0666, false, true, NULL, NULL, NULL, &f3) == 0); + assert_se(journal_file_open("test-seal.journal", O_RDWR|O_CREAT, 0666, false, true, NULL, NULL, NULL, NULL, &f3) == 0); - assert_se(journal_file_open("test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, true, NULL, NULL, NULL, &f4) == 0); + assert_se(journal_file_open("test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, true, NULL, NULL, NULL, NULL, &f4) == 0); journal_file_print_header(f1); puts(""); @@ -158,10 +158,10 @@ static void test_empty(void) { assert_se(rm_rf(t, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); } - journal_file_close(f1); - journal_file_close(f2); - journal_file_close(f3); - journal_file_close(f4); + (void) journal_file_close(f1); + (void) journal_file_close(f2); + (void) journal_file_close(f3); + (void) journal_file_close(f4); } int main(int argc, char *argv[]) { diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 9633e46ce0..fdd8c05e9c 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1397,7 +1397,7 @@ _public_ const char *sd_device_get_tag_first(sd_device *device) { device->tags_iterator_generation = device->tags_generation; device->tags_iterator = ITERATOR_FIRST; - set_iterate(device->tags, &device->tags_iterator, &v); + (void) set_iterate(device->tags, &device->tags_iterator, &v); return v; } @@ -1411,7 +1411,7 @@ _public_ const char *sd_device_get_tag_next(sd_device *device) { if (device->tags_iterator_generation != device->tags_generation) return NULL; - set_iterate(device->tags, &device->tags_iterator, &v); + (void) set_iterate(device->tags, &device->tags_iterator, &v); return v; } @@ -1425,7 +1425,7 @@ _public_ const char *sd_device_get_devlink_first(sd_device *device) { device->devlinks_iterator_generation = device->devlinks_generation; device->devlinks_iterator = ITERATOR_FIRST; - set_iterate(device->devlinks, &device->devlinks_iterator, &v); + (void) set_iterate(device->devlinks, &device->devlinks_iterator, &v); return v; } @@ -1439,7 +1439,7 @@ _public_ const char *sd_device_get_devlink_next(sd_device *device) { if (device->devlinks_iterator_generation != device->devlinks_generation) return NULL; - set_iterate(device->devlinks, &device->devlinks_iterator, &v); + (void) set_iterate(device->devlinks, &device->devlinks_iterator, &v); return v; } @@ -1606,7 +1606,7 @@ _public_ const char *sd_device_get_sysattr_first(sd_device *device) { device->sysattrs_iterator = ITERATOR_FIRST; - set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); + (void) set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); return v; } @@ -1618,7 +1618,7 @@ _public_ const char *sd_device_get_sysattr_next(sd_device *device) { if (!device->sysattrs_read) return NULL; - set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); + (void) set_iterate(device->sysattrs, &device->sysattrs_iterator, &v); return v; } diff --git a/src/test/test-siphash24.c b/src/test/test-siphash24.c index caae911f30..b74b7ad2dd 100644 --- a/src/test/test-siphash24.c +++ b/src/test/test-siphash24.c @@ -22,9 +22,9 @@ #define ITERATIONS 10000000ULL -static int do_test(const uint8_t *in, size_t len, const uint8_t *key) { +static void do_test(const uint8_t *in, size_t len, const uint8_t *key) { struct siphash state = {}; - uint64_t out = 0; + uint64_t out; unsigned i, j; out = siphash24(in, len, key); @@ -60,7 +60,46 @@ static int do_test(const uint8_t *in, size_t len, const uint8_t *key) { assert_se(out == 0xa129ca6149be45e5); } } - return 0; +} + +static void test_short_hashes(void) { + const uint8_t one[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16 }; + const uint8_t key[16] = { 0x22, 0x24, 0x41, 0x22, 0x55, 0x77, 0x88, 0x07, + 0x23, 0x09, 0x23, 0x14, 0x0c, 0x33, 0x0e, 0x0f}; + uint8_t two[sizeof one] = {}; + + struct siphash state1 = {}, state2 = {}; + unsigned i, j; + + siphash24_init(&state1, key); + siphash24_init(&state2, key); + + /* hashing 1, 2, 3, 4, 5, ..., 16 bytes, with the byte after the buffer different */ + for (i = 1; i <= sizeof one; i++) { + siphash24_compress(one, i, &state1); + + two[i-1] = one[i-1]; + siphash24_compress(two, i, &state2); + + assert_se(memcmp(&state1, &state2, sizeof state1) == 0); + } + + /* hashing n and 1, n and 2, n and 3, ..., n-1 and 1, n-2 and 2, ... */ + for (i = sizeof one; i > 0; i--) { + zero(two); + + for (j = 1; j <= sizeof one; j++) { + siphash24_compress(one, i, &state1); + siphash24_compress(one, j, &state1); + + siphash24_compress(one, i, &state2); + two[j-1] = one[j-1]; + siphash24_compress(two, j, &state2); + + assert_se(memcmp(&state1, &state2, sizeof state1) == 0); + } + } } /* see https://131002.net/siphash/siphash.pdf, Appendix A */ @@ -80,4 +119,6 @@ int main(int argc, char *argv[]) { do_test(in_buf + 2, sizeof(in), key); memcpy(in_buf + 4, in, sizeof(in)); do_test(in_buf + 4, sizeof(in), key); + + test_short_hashes(); } diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index c06ace09cf..8470456d4c 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -33,9 +33,11 @@ #include "conf-files.h" #include "escape.h" #include "fd-util.h" +#include "fs-util.h" #include "glob-util.h" #include "path-util.h" #include "stat-util.h" +#include "stdio-util.h" #include "strbuf.h" #include "string-util.h" #include "strv.h" @@ -686,41 +688,31 @@ static int import_parent_into_properties(struct udev_device *dev, const char *fi return 0; } -static int attr_subst_subdir(char *attr, size_t len) { - bool found = false; +static void attr_subst_subdir(char *attr, size_t len) { + const char *pos, *tail, *path; + _cleanup_closedir_ DIR *dir = NULL; + struct dirent *dent; - if (strstr(attr, "/*/")) { - char *pos; - char dirname[UTIL_PATH_SIZE]; - const char *tail; - DIR *dir; + pos = strstr(attr, "/*/"); + if (!pos) + return; - strscpy(dirname, sizeof(dirname), attr); - pos = strstr(dirname, "/*/"); - if (pos == NULL) - return -1; - pos[0] = '\0'; - tail = &pos[2]; - dir = opendir(dirname); - if (dir != NULL) { - struct dirent *dent; + tail = pos + 2; + path = strndupa(attr, pos - attr + 1); /* include slash at end */ + dir = opendir(path); + if (dir == NULL) + return; - for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) { - struct stat stats; + for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) + if (dent->d_name[0] != '.') { + char n[strlen(dent->d_name) + 1 + strlen(tail) + 1]; - if (dent->d_name[0] == '.') - continue; - strscpyl(attr, len, dirname, "/", dent->d_name, tail, NULL); - if (stat(attr, &stats) == 0) { - found = true; - break; - } + strscpyl(n, sizeof n, dent->d_name, "/", tail, NULL); + if (faccessat(dirfd(dir), n, F_OK, 0)) { + strscpyl(attr, len, path, n, NULL); + break; } - closedir(dir); } - } - - return found; } static int get_key(struct udev *udev, char **line, char **key, enum operation_type *op, char **value) { @@ -831,12 +823,13 @@ static const char *get_key_attribute(struct udev *udev, char *str) { return NULL; } -static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, - enum operation_type op, - const char *value, const void *data) { - struct token *token = &rule_tmp->token[rule_tmp->token_cur]; +static void rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, + enum operation_type op, + const char *value, const void *data) { + struct token *token = rule_tmp->token + rule_tmp->token_cur; const char *attr = NULL; + assert(rule_tmp->token_cur < ELEMENTSOF(rule_tmp->token)); memzero(token, sizeof(struct token)); switch (type) { @@ -919,8 +912,7 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, case TK_M_MAX: case TK_END: case TK_UNSET: - log_error("wrong type %u", type); - return -1; + assert_not_reached("wrong type"); } if (value != NULL && type < TK_M_MAX) { @@ -969,11 +961,6 @@ static int rule_add_key(struct rule_tmp *rule_tmp, enum token_type type, token->key.type = type; token->key.op = op; rule_tmp->token_cur++; - if (rule_tmp->token_cur >= ELEMENTSOF(rule_tmp->token)) { - log_error("temporary rule array too small"); - return -1; - } - return 0; } static int sort_token(struct udev_rules *rules, struct rule_tmp *rule_tmp) { @@ -1010,8 +997,13 @@ static int sort_token(struct udev_rules *rules, struct rule_tmp *rule_tmp) { return 0; } -static int add_rule(struct udev_rules *rules, char *line, - const char *filename, unsigned int filename_off, unsigned int lineno) { +#define LOG_RULE_ERROR(fmt, ...) log_error("Invalid rule %s:%u: " fmt, filename, lineno, ##__VA_ARGS__) +#define LOG_RULE_WARNING(fmt, ...) log_warning("%s:%u: " fmt, filename, lineno, ##__VA_ARGS__) +#define LOG_RULE_DEBUG(fmt, ...) log_debug("%s:%u: " fmt, filename, lineno, ##__VA_ARGS__) +#define LOG_AND_RETURN(fmt, ...) { LOG_RULE_ERROR(fmt, __VA_ARGS__); return; } + +static void add_rule(struct udev_rules *rules, char *line, + const char *filename, unsigned int filename_off, unsigned int lineno) { char *linepos; const char *attr; struct rule_tmp rule_tmp = { @@ -1052,427 +1044,322 @@ static int add_rule(struct udev_rules *rules, char *line, break; } + if (rule_tmp.token_cur >= ELEMENTSOF(rule_tmp.token)) + LOG_AND_RETURN("temporary rule array too small, aborting event processing with %u items", rule_tmp.token_cur); + if (streq(key, "ACTION")) { - if (op > OP_MATCH_MAX) { - log_error("invalid ACTION operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_ACTION, op, value, NULL); - continue; - } - if (streq(key, "DEVPATH")) { - if (op > OP_MATCH_MAX) { - log_error("invalid DEVPATH operation"); - goto invalid; - } + } else if (streq(key, "DEVPATH")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_DEVPATH, op, value, NULL); - continue; - } - if (streq(key, "KERNEL")) { - if (op > OP_MATCH_MAX) { - log_error("invalid KERNEL operation"); - goto invalid; - } + } else if (streq(key, "KERNEL")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_KERNEL, op, value, NULL); - continue; - } - if (streq(key, "SUBSYSTEM")) { - if (op > OP_MATCH_MAX) { - log_error("invalid SUBSYSTEM operation"); - goto invalid; - } + } else if (streq(key, "SUBSYSTEM")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + /* bus, class, subsystem events should all be the same */ - if (streq(value, "subsystem") || - streq(value, "bus") || - streq(value, "class")) { - if (streq(value, "bus") || streq(value, "class")) - log_error("'%s' must be specified as 'subsystem' " - "please fix it in %s:%u", value, filename, lineno); + if (STR_IN_SET(value, "subsystem", "bus", "class")) { + if (!streq(value, "subsystem")) + LOG_RULE_WARNING("'%s' must be specified as 'subsystem'; please fix", value); + rule_add_key(&rule_tmp, TK_M_SUBSYSTEM, op, "subsystem|class|bus", NULL); } else rule_add_key(&rule_tmp, TK_M_SUBSYSTEM, op, value, NULL); - continue; - } - if (streq(key, "DRIVER")) { - if (op > OP_MATCH_MAX) { - log_error("invalid DRIVER operation"); - goto invalid; - } + } else if (streq(key, "DRIVER")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_DRIVER, op, value, NULL); - continue; - } - if (startswith(key, "ATTR{")) { + } else if (startswith(key, "ATTR{")) { attr = get_key_attribute(rules->udev, key + strlen("ATTR")); - if (attr == NULL) { - log_error("error parsing ATTR attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid ATTR operation"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ATTR"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "ATTR"); + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_ATTR, op, value, attr); else rule_add_key(&rule_tmp, TK_A_ATTR, op, value, attr); - continue; - } - if (startswith(key, "SYSCTL{")) { + } else if (startswith(key, "SYSCTL{")) { attr = get_key_attribute(rules->udev, key + strlen("SYSCTL")); - if (attr == NULL) { - log_error("error parsing SYSCTL attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid SYSCTL operation"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ATTR"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "ATTR"); + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_SYSCTL, op, value, attr); else rule_add_key(&rule_tmp, TK_A_SYSCTL, op, value, attr); - continue; - } - if (startswith(key, "SECLABEL{")) { + } else if (startswith(key, "SECLABEL{")) { attr = get_key_attribute(rules->udev, key + strlen("SECLABEL")); - if (!attr) { - log_error("error parsing SECLABEL attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid SECLABEL operation"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "SECLABEL"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "SECLABEL"); rule_add_key(&rule_tmp, TK_A_SECLABEL, op, value, attr); - continue; - } - if (streq(key, "KERNELS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid KERNELS operation"); - goto invalid; - } + } else if (streq(key, "KERNELS")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_KERNELS, op, value, NULL); - continue; - } - if (streq(key, "SUBSYSTEMS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid SUBSYSTEMS operation"); - goto invalid; - } + } else if (streq(key, "SUBSYSTEMS")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_SUBSYSTEMS, op, value, NULL); - continue; - } - if (streq(key, "DRIVERS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid DRIVERS operation"); - goto invalid; - } + } else if (streq(key, "DRIVERS")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_DRIVERS, op, value, NULL); - continue; - } - if (startswith(key, "ATTRS{")) { - if (op > OP_MATCH_MAX) { - log_error("invalid ATTRS operation"); - goto invalid; - } + } else if (startswith(key, "ATTRS{")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", "ATTRS"); + attr = get_key_attribute(rules->udev, key + strlen("ATTRS")); - if (attr == NULL) { - log_error("error parsing ATTRS attribute"); - goto invalid; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ATTRS"); + if (startswith(attr, "device/")) - log_error("the 'device' link may not be available in a future kernel, " - "please fix it in %s:%u", filename, lineno); - else if (strstr(attr, "../") != NULL) - log_error("do not reference parent sysfs directories directly, " - "it may break with a future kernel, please fix it in %s:%u", filename, lineno); + LOG_RULE_WARNING("'device' link may not be available in future kernels; please fix"); + if (strstr(attr, "../") != NULL) + LOG_RULE_WARNING("direct reference to parent sysfs directory, may break in future kernels; please fix"); rule_add_key(&rule_tmp, TK_M_ATTRS, op, value, attr); - continue; - } - if (streq(key, "TAGS")) { - if (op > OP_MATCH_MAX) { - log_error("invalid TAGS operation"); - goto invalid; - } + } else if (streq(key, "TAGS")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_TAGS, op, value, NULL); - continue; - } - if (startswith(key, "ENV{")) { + } else if (startswith(key, "ENV{")) { attr = get_key_attribute(rules->udev, key + strlen("ENV")); - if (attr == NULL) { - log_error("error parsing ENV attribute"); - goto invalid; - } - if (op == OP_REMOVE) { - log_error("invalid ENV operation"); - goto invalid; - } - if (op < OP_MATCH_MAX) { - if (rule_add_key(&rule_tmp, TK_M_ENV, op, value, attr) != 0) - goto invalid; - } else { - static const char *blacklist[] = { - "ACTION", - "SUBSYSTEM", - "DEVTYPE", - "MAJOR", - "MINOR", - "DRIVER", - "IFINDEX", - "DEVNAME", - "DEVLINKS", - "DEVPATH", - "TAGS", - }; - unsigned int i; - - for (i = 0; i < ELEMENTSOF(blacklist); i++) { - if (!streq(attr, blacklist[i])) - continue; - log_error("invalid ENV attribute, '%s' can not be set %s:%u", attr, filename, lineno); - goto invalid; - } - if (rule_add_key(&rule_tmp, TK_A_ENV, op, value, attr) != 0) - goto invalid; - } - continue; - } + if (attr == NULL) + LOG_AND_RETURN("error parsing %s attribute", "ENV"); + + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "ENV"); - if (streq(key, "TAG")) { + if (op < OP_MATCH_MAX) + rule_add_key(&rule_tmp, TK_M_ENV, op, value, attr); + else { + if (STR_IN_SET(attr, + "ACTION", + "SUBSYSTEM", + "DEVTYPE", + "MAJOR", + "MINOR", + "DRIVER", + "IFINDEX", + "DEVNAME", + "DEVLINKS", + "DEVPATH", + "TAGS")) + LOG_AND_RETURN("invalid ENV attribute, '%s' cannot be set", attr); + + rule_add_key(&rule_tmp, TK_A_ENV, op, value, attr); + } + + } else if (streq(key, "TAG")) { if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_TAG, op, value, NULL); else rule_add_key(&rule_tmp, TK_A_TAG, op, value, NULL); - continue; - } - if (streq(key, "PROGRAM")) { - if (op == OP_REMOVE) { - log_error("invalid PROGRAM operation"); - goto invalid; - } + } else if (streq(key, "PROGRAM")) { + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_PROGRAM, op, value, NULL); - continue; - } - if (streq(key, "RESULT")) { - if (op > OP_MATCH_MAX) { - log_error("invalid RESULT operation"); - goto invalid; - } + } else if (streq(key, "RESULT")) { + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_M_RESULT, op, value, NULL); - continue; - } - if (startswith(key, "IMPORT")) { + } else if (startswith(key, "IMPORT")) { attr = get_key_attribute(rules->udev, key + strlen("IMPORT")); if (attr == NULL) { - log_error("IMPORT{} type missing, ignoring IMPORT %s:%u", filename, lineno); + LOG_RULE_WARNING("ignoring IMPORT{} with missing type"); continue; } - if (op == OP_REMOVE) { - log_error("invalid IMPORT operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "IMPORT"); + if (streq(attr, "program")) { /* find known built-in command */ if (value[0] != '/') { - enum udev_builtin_cmd cmd; + const enum udev_builtin_cmd cmd = udev_builtin_lookup(value); - cmd = udev_builtin_lookup(value); if (cmd < UDEV_BUILTIN_MAX) { - log_debug("IMPORT found builtin '%s', replacing %s:%u", - value, filename, lineno); + LOG_RULE_DEBUG("IMPORT found builtin '%s', replacing", value); rule_add_key(&rule_tmp, TK_M_IMPORT_BUILTIN, op, value, &cmd); continue; } } rule_add_key(&rule_tmp, TK_M_IMPORT_PROG, op, value, NULL); } else if (streq(attr, "builtin")) { - enum udev_builtin_cmd cmd = udev_builtin_lookup(value); + const enum udev_builtin_cmd cmd = udev_builtin_lookup(value); - if (cmd < UDEV_BUILTIN_MAX) - rule_add_key(&rule_tmp, TK_M_IMPORT_BUILTIN, op, value, &cmd); + if (cmd >= UDEV_BUILTIN_MAX) + LOG_RULE_WARNING("IMPORT{builtin} '%s' unknown", value); else - log_error("IMPORT{builtin}: '%s' unknown %s:%u", value, filename, lineno); - } else if (streq(attr, "file")) { + rule_add_key(&rule_tmp, TK_M_IMPORT_BUILTIN, op, value, &cmd); + } else if (streq(attr, "file")) rule_add_key(&rule_tmp, TK_M_IMPORT_FILE, op, value, NULL); - } else if (streq(attr, "db")) { + else if (streq(attr, "db")) rule_add_key(&rule_tmp, TK_M_IMPORT_DB, op, value, NULL); - } else if (streq(attr, "cmdline")) { + else if (streq(attr, "cmdline")) rule_add_key(&rule_tmp, TK_M_IMPORT_CMDLINE, op, value, NULL); - } else if (streq(attr, "parent")) { + else if (streq(attr, "parent")) rule_add_key(&rule_tmp, TK_M_IMPORT_PARENT, op, value, NULL); - } else - log_error("IMPORT{} unknown type, ignoring IMPORT %s:%u", filename, lineno); - continue; - } + else + LOG_RULE_ERROR("ignoring unknown %s{} type '%s'", "IMPORT", attr); - if (startswith(key, "TEST")) { + } else if (startswith(key, "TEST")) { mode_t mode = 0; - if (op > OP_MATCH_MAX) { - log_error("invalid TEST operation"); - goto invalid; - } + if (op > OP_MATCH_MAX) + LOG_AND_RETURN("invalid %s operation", "TEST"); + attr = get_key_attribute(rules->udev, key + strlen("TEST")); if (attr != NULL) { mode = strtol(attr, NULL, 8); rule_add_key(&rule_tmp, TK_M_TEST, op, value, &mode); - } else { + } else rule_add_key(&rule_tmp, TK_M_TEST, op, value, NULL); - } - continue; - } - if (startswith(key, "RUN")) { + } else if (startswith(key, "RUN")) { attr = get_key_attribute(rules->udev, key + strlen("RUN")); if (attr == NULL) attr = "program"; - if (op == OP_REMOVE) { - log_error("invalid RUN operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", "RUN"); if (streq(attr, "builtin")) { - enum udev_builtin_cmd cmd = udev_builtin_lookup(value); + const enum udev_builtin_cmd cmd = udev_builtin_lookup(value); if (cmd < UDEV_BUILTIN_MAX) rule_add_key(&rule_tmp, TK_A_RUN_BUILTIN, op, value, &cmd); else - log_error("RUN{builtin}: '%s' unknown %s:%u", value, filename, lineno); + LOG_RULE_ERROR("RUN{builtin}: '%s' unknown", value); } else if (streq(attr, "program")) { - enum udev_builtin_cmd cmd = UDEV_BUILTIN_MAX; + const enum udev_builtin_cmd cmd = UDEV_BUILTIN_MAX; rule_add_key(&rule_tmp, TK_A_RUN_PROGRAM, op, value, &cmd); - } else { - log_error("RUN{} unknown type, ignoring RUN %s:%u", filename, lineno); - } + } else + LOG_RULE_ERROR("ignoring unknown %s{} type '%s'", "RUN", attr); - continue; - } + } else if (streq(key, "LABEL")) { + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); - if (streq(key, "LABEL")) { - if (op == OP_REMOVE) { - log_error("invalid LABEL operation"); - goto invalid; - } rule_tmp.rule.rule.label_off = rules_add_string(rules, value); - continue; - } - if (streq(key, "GOTO")) { - if (op == OP_REMOVE) { - log_error("invalid GOTO operation"); - goto invalid; - } + } else if (streq(key, "GOTO")) { + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + rule_add_key(&rule_tmp, TK_A_GOTO, 0, value, NULL); - continue; - } - if (startswith(key, "NAME")) { - if (op == OP_REMOVE) { - log_error("invalid NAME operation"); - goto invalid; - } - if (op < OP_MATCH_MAX) { + } else if (startswith(key, "NAME")) { + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_NAME, op, value, NULL); - } else { + else { if (streq(value, "%k")) { - log_error("NAME=\"%%k\" is ignored, because it breaks kernel supplied names, " - "please remove it from %s:%u\n", filename, lineno); + LOG_RULE_WARNING("NAME=\"%%k\" is ignored, because it breaks kernel supplied names; please remove"); continue; } - if (value[0] == '\0') { - log_debug("NAME=\"\" is ignored, because udev will not delete any device nodes, " - "please remove it from %s:%u\n", filename, lineno); + if (isempty(value)) { + LOG_RULE_DEBUG("NAME=\"\" is ignored, because udev will not delete any device nodes; please remove"); continue; } rule_add_key(&rule_tmp, TK_A_NAME, op, value, NULL); } rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "SYMLINK")) { - if (op == OP_REMOVE) { - log_error("invalid SYMLINK operation"); - goto invalid; - } + } else if (streq(key, "SYMLINK")) { + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); + if (op < OP_MATCH_MAX) rule_add_key(&rule_tmp, TK_M_DEVLINK, op, value, NULL); else rule_add_key(&rule_tmp, TK_A_DEVLINK, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "OWNER")) { + } else if (streq(key, "OWNER")) { uid_t uid; char *endptr; - if (op == OP_REMOVE) { - log_error("invalid OWNER operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); uid = strtoul(value, &endptr, 10); - if (endptr[0] == '\0') { + if (endptr[0] == '\0') rule_add_key(&rule_tmp, TK_A_OWNER_ID, op, NULL, &uid); - } else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { + else if (rules->resolve_names > 0 && strchr("$%", value[0]) == NULL) { uid = add_uid(rules, value); rule_add_key(&rule_tmp, TK_A_OWNER_ID, op, NULL, &uid); } else if (rules->resolve_names >= 0) rule_add_key(&rule_tmp, TK_A_OWNER, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "GROUP")) { + } else if (streq(key, "GROUP")) { gid_t gid; char *endptr; - if (op == OP_REMOVE) { - log_error("invalid GROUP operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); gid = strtoul(value, &endptr, 10); - if (endptr[0] == '\0') { + if (endptr[0] == '\0') rule_add_key(&rule_tmp, TK_A_GROUP_ID, op, NULL, &gid); - } else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { + else if ((rules->resolve_names > 0) && strchr("$%", value[0]) == NULL) { gid = add_gid(rules, value); rule_add_key(&rule_tmp, TK_A_GROUP_ID, op, NULL, &gid); } else if (rules->resolve_names >= 0) rule_add_key(&rule_tmp, TK_A_GROUP, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "MODE")) { + } else if (streq(key, "MODE")) { mode_t mode; char *endptr; - if (op == OP_REMOVE) { - log_error("invalid MODE operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); mode = strtol(value, &endptr, 8); if (endptr[0] == '\0') @@ -1480,27 +1367,23 @@ static int add_rule(struct udev_rules *rules, char *line, else rule_add_key(&rule_tmp, TK_A_MODE, op, value, NULL); rule_tmp.rule.rule.can_set_name = true; - continue; - } - if (streq(key, "OPTIONS")) { + } else if (streq(key, "OPTIONS")) { const char *pos; - if (op == OP_REMOVE) { - log_error("invalid OPTIONS operation"); - goto invalid; - } + if (op == OP_REMOVE) + LOG_AND_RETURN("invalid %s operation", key); pos = strstr(value, "link_priority="); if (pos != NULL) { - int prio = atoi(&pos[strlen("link_priority=")]); + int prio = atoi(pos + strlen("link_priority=")); rule_add_key(&rule_tmp, TK_A_DEVLINK_PRIO, op, NULL, &prio); } pos = strstr(value, "string_escape="); if (pos != NULL) { - pos = &pos[strlen("string_escape=")]; + pos += strlen("string_escape="); if (startswith(pos, "none")) rule_add_key(&rule_tmp, TK_A_STRING_ESCAPE_NONE, op, NULL, NULL); else if (startswith(pos, "replace")) @@ -1527,30 +1410,19 @@ static int add_rule(struct udev_rules *rules, char *line, pos = strstr(value, "static_node="); if (pos != NULL) { - rule_add_key(&rule_tmp, TK_A_STATIC_NODE, op, &pos[strlen("static_node=")], NULL); + pos += strlen("static_node="); + rule_add_key(&rule_tmp, TK_A_STATIC_NODE, op, pos, NULL); rule_tmp.rule.rule.has_static_node = true; } - continue; - } - - log_error("unknown key '%s' in %s:%u", key, filename, lineno); - goto invalid; + } else + LOG_AND_RETURN("unknown key '%s'", key); } - /* add rule token */ + /* add rule token and sort tokens */ rule_tmp.rule.rule.token_count = 1 + rule_tmp.token_cur; - if (add_token(rules, &rule_tmp.rule) != 0) - goto invalid; - - /* add tokens to list, sorted by type */ - if (sort_token(rules, &rule_tmp) != 0) - goto invalid; - - return 0; -invalid: - log_error("invalid rule '%s:%u'", filename, lineno); - return -1; + if (add_token(rules, &rule_tmp.rule) != 0 || sort_token(rules, &rule_tmp) != 0) + LOG_RULE_ERROR("failed to add rule token"); } static int parse_file(struct udev_rules *rules, const char *filename) { @@ -1849,18 +1721,18 @@ enum escape_type { ESCAPE_REPLACE, }; -int udev_rules_apply_to_event(struct udev_rules *rules, - struct udev_event *event, - usec_t timeout_usec, - usec_t timeout_warn_usec, - struct udev_list *properties_list) { +void udev_rules_apply_to_event(struct udev_rules *rules, + struct udev_event *event, + usec_t timeout_usec, + usec_t timeout_warn_usec, + struct udev_list *properties_list) { struct token *cur; struct token *rule; enum escape_type esc = ESCAPE_UNSET; bool can_set_name; if (rules->tokens == NULL) - return -1; + return; can_set_name = ((!streq(udev_device_get_action(event->dev), "remove")) && (major(udev_device_get_devnum(event->dev)) > 0 || @@ -2168,7 +2040,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, break; } case TK_M_IMPORT_CMDLINE: { - FILE *f; + _cleanup_fclose_ FILE *f = NULL; bool imported = false; f = fopen("/proc/cmdline", "re"); @@ -2181,12 +2053,12 @@ int udev_rules_apply_to_event(struct udev_rules *rules, pos = strstr(cmdline, key); if (pos != NULL) { + imported = true; pos += strlen(key); - if (pos[0] == '\0' || isspace(pos[0])) { + if (pos[0] == '\0' || isspace(pos[0])) /* we import simple flags as 'FLAG=1' */ udev_device_add_property(event->dev, key, "1"); - imported = true; - } else if (pos[0] == '=') { + else if (pos[0] == '=') { const char *value; pos++; @@ -2195,11 +2067,9 @@ int udev_rules_apply_to_event(struct udev_rules *rules, pos++; pos[0] = '\0'; udev_device_add_property(event->dev, key, value); - imported = true; } } } - fclose(f); } if (!imported && cur->key.op != OP_NOMATCH) goto nomatch; @@ -2428,13 +2298,17 @@ int udev_rules_apply_to_event(struct udev_rules *rules, log_debug("%i character(s) replaced", count); } if (major(udev_device_get_devnum(event->dev)) && - (!streq(name_str, udev_device_get_devnode(event->dev) + strlen("/dev/")))) { - log_error("NAME=\"%s\" ignored, kernel device nodes " - "can not be renamed; please fix it in %s:%u\n", name, - rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); + !streq(name_str, udev_device_get_devnode(event->dev) + strlen("/dev/"))) { + log_error("NAME=\"%s\" ignored, kernel device nodes cannot be renamed; please fix it in %s:%u\n", + name, + rules_str(rules, rule->rule.filename_off), + rule->rule.filename_line); break; } - free_and_strdup(&event->name, name_str); + if (free_and_strdup(&event->name, name_str) < 0) { + log_oom(); + return; + } log_debug("NAME '%s' %s:%u", event->name, rules_str(rules, rule->rule.filename_off), @@ -2491,7 +2365,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, const char *key_name = rules_str(rules, cur->key.attr_off); char attr[UTIL_PATH_SIZE]; char value[UTIL_NAME_SIZE]; - FILE *f; + _cleanup_fclose_ FILE *f = NULL; if (util_resolve_subsys_kernel(event->udev, key_name, attr, sizeof(attr), 0) != 0) strscpyl(attr, sizeof(attr), udev_device_get_syspath(event->dev), "/", key_name, NULL); @@ -2502,13 +2376,10 @@ int udev_rules_apply_to_event(struct udev_rules *rules, rules_str(rules, rule->rule.filename_off), rule->rule.filename_line); f = fopen(attr, "we"); - if (f != NULL) { - if (fprintf(f, "%s", value) <= 0) - log_error_errno(errno, "error writing ATTR{%s}: %m", attr); - fclose(f); - } else { + if (f == NULL) log_error_errno(errno, "error opening ATTR{%s} for writing: %m", attr); - } + else if (fprintf(f, "%s", value) <= 0) + log_error_errno(errno, "error writing ATTR{%s}: %m", attr); break; } case TK_A_SYSCTL: { @@ -2546,7 +2417,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, cur = &rules->tokens[cur->key.rule_goto]; continue; case TK_END: - return 0; + return; case TK_M_PARENTS_MIN: case TK_M_PARENTS_MAX: @@ -2574,7 +2445,7 @@ int udev_rules_apply_static_dev_perms(struct udev_rules *rules) { char **t; FILE *f = NULL; _cleanup_free_ char *path = NULL; - int r = 0; + int r; if (rules->tokens == NULL) return 0; @@ -2645,8 +2516,6 @@ int udev_rules_apply_static_dev_perms(struct udev_rules *rules) { if (r < 0 && errno != EEXIST) return log_error_errno(errno, "failed to create symlink %s -> %s: %m", tag_symlink, device_node); - else - r = 0; } } @@ -2698,12 +2567,11 @@ finish: fflush(f); fchmod(fileno(f), 0644); if (ferror(f) || rename(path, "/run/udev/static_node-tags") < 0) { - r = -errno; - unlink("/run/udev/static_node-tags"); - unlink(path); + unlink_noerrno("/run/udev/static_node-tags"); + unlink_noerrno(path); + return -errno; } - fclose(f); } - return r; + return 0; } diff --git a/src/udev/udev.h b/src/udev/udev.h index 1f9c8120c0..655880346b 100644 --- a/src/udev/udev.h +++ b/src/udev/udev.h @@ -71,9 +71,9 @@ struct udev_rules; struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names); struct udev_rules *udev_rules_unref(struct udev_rules *rules); bool udev_rules_check_timestamp(struct udev_rules *rules); -int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, - usec_t timeout_usec, usec_t timeout_warn_usec, - struct udev_list *properties_list); +void udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, + usec_t timeout_usec, usec_t timeout_warn_usec, + struct udev_list *properties_list); int udev_rules_apply_static_dev_perms(struct udev_rules *rules); /* udev-event.c */ |