diff --git a/ChangeLog b/ChangeLog index 1b1b8b3..27ce806 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2018-01-15 Martin Pärtel + + * Added option --no-user-group-precaching (issue #62). + 2017-11-30 Martin Pärtel * Added options --delete-deny and --rename-deny as suggested by @roojs. diff --git a/src/bindfs.1 b/src/bindfs.1 index d47720f..f5f6810 100644 --- a/src/bindfs.1 +++ b/src/bindfs.1 @@ -360,6 +360,14 @@ ioctl. This may be a security concern, especially when mounting as root. .B \-\-block\-devices\-as\-files, \-o block\-devices\-as\-files Shows block devices as regular files. +.TP +.B \-\-no\-user\-group\-precaching, \-o no\-user\-group\-precaching +Disables the default behaviour of caching users and their group memberships +at startup. Instead they are read and then cached indefinitely on-demand. +Sending bindfs \fBSIGUSR1\fP will clear this cache in either case. + +Currently this cache is only used by \fB\-\-mirror\fP and \fB\-\-mirror\-only\fP. + .TP .B \-\-multithreaded, \-o multithreaded Run bindfs in multithreaded mode. While bindfs is designed to be diff --git a/src/bindfs.c b/src/bindfs.c index 6cd7c60..e8226ec 100644 --- a/src/bindfs.c +++ b/src/bindfs.c @@ -1486,6 +1486,7 @@ static void print_usage(const char *progname) " --resolve-symlinks Resolve symbolic links.\n" " --resolved-symlink-deletion=... Decide how to delete resolved symlinks.\n" " --block-devices-as-files Show block devices as regular files.\n" + " --no-user-group-precaching Don't read all users and groups at startup.\n" " --multithreaded Enable multithreaded mode. See man page\n" " for security issue with current implementation.\n" "\n" @@ -1846,11 +1847,14 @@ static void setup_signal_handling() static void signal_handler(int sig) { - invalidate_user_cache(); + invalidate_user_caches(); } static void atexit_func() { + // This cleanup is mostly unnecessary, but we do it anyway to + // reduce the "still reachable" allocations that Valgrind shows. + free(settings.mntsrc); free(settings.mntdest); free(settings.original_working_dir); @@ -1879,6 +1883,8 @@ static void atexit_func() settings.mirrored_users = NULL; free(settings.mirrored_members); settings.mirrored_members = NULL; + + clear_user_caches(); } int main(int argc, char *argv[]) @@ -1903,6 +1909,7 @@ int main(int argc, char *argv[]) char *chmod_filter; char *resolved_symlink_deletion; int no_allow_other; + int no_user_group_precaching; int multithreaded; char *uid_offset; char *gid_offset; @@ -1974,13 +1981,11 @@ int main(int argc, char *argv[]) OPT2("--enable-lock-forwarding", "enable-lock-forwarding", OPTKEY_ENABLE_LOCK_FORWARDING), OPT2("--disable-lock-forwarding", "disable-lock-forwarding", OPTKEY_DISABLE_LOCK_FORWARDING), OPT2("--enable-ioctl", "enable-ioctl", OPTKEY_ENABLE_IOCTL), + OPT_OFFSET2("--no-user-group-precaching", "no-user-group-precaching", no_user_group_precaching, -1), OPT_OFFSET2("--multithreaded", "multithreaded", multithreaded, -1), OPT_OFFSET2("--uid-offset=%s", "uid-offset=%s", uid_offset, 0), OPT_OFFSET2("--gid-offset=%s", "gid-offset=%s", gid_offset, 0), - - - - + FUSE_OPT_END }; @@ -2176,6 +2181,11 @@ int main(int argc, char *argv[]) } } + /* Precache entire user/group database for use by --mirror by default */ + if (!od.no_user_group_precaching) { + rebuild_user_caches(); + } + /* Parse permission bits */ if (od.perms) { if (add_chmod_rules_to_permchain(od.perms, settings.permchain) != 0) { diff --git a/src/misc.c b/src/misc.c index eae81f0..32a6b19 100644 --- a/src/misc.c +++ b/src/misc.c @@ -135,6 +135,30 @@ const char *my_dirname(char *path) } } +void insertion_sort_last(void *base, size_t nmemb, size_t size, int (*compar)(const void*, const void*)) +{ + void *const last = base + (nmemb - 1) * size; + void *const entry = malloc(size); + memcpy(entry, last, size); + + // Find the location to insert at. + int n = 0; + void *p = last - size; + while (p >= base && compar(entry, p) < 0) { + p -= size; + n += 1; + } + + // We found a *p that's larger or we went below the start of the array, so go back one step. + p += size; + // No need to adjust n, it's now the correct number of elements to shift forward + + // Insert at *p. + memmove(p + size, p, n * size); + memcpy(p, entry, size); + free(entry); +} + void grow_array_impl(void **array, int *capacity, int member_size) { int new_cap = *capacity; diff --git a/src/misc.h b/src/misc.h index 6bcf865..f49ad9e 100644 --- a/src/misc.h +++ b/src/misc.h @@ -20,6 +20,7 @@ #ifndef INC_BINDFS_MISC_H #define INC_BINDFS_MISC_H +#include /* Counts the number of times ch occurs in s. */ int count_chars(const char *s, char ch); @@ -49,6 +50,10 @@ const char *my_basename(const char *path); Otherwise, returns ".". */ const char *my_dirname(char *path); +/* Sorts an array that is otherwise sorted, but whose last element may be in the wrong place. + The signature is the same as qsort's. */ +void insertion_sort_last(void *base, size_t nmemb, size_t size, int (*compar)(const void*, const void*)); + /* Reallocs `*array` (may be NULL) to be at least one larger than `*capacity` (may be 0) and stores the new capacity in `*capacity`. */ diff --git a/src/userinfo.c b/src/userinfo.c index 23c54c1..8302595 100644 --- a/src/userinfo.c +++ b/src/userinfo.c @@ -39,9 +39,11 @@ struct gid_cache_entry { static pthread_rwlock_t cache_lock = PTHREAD_RWLOCK_INITIALIZER; -static struct uid_cache_entry *uid_cache = NULL; +static struct uid_cache_entry *uid_cache_by_uid = NULL; +static struct uid_cache_entry *uid_cache_by_name = NULL; static int uid_cache_size = 0; -static int uid_cache_capacity = 0; +static int uid_cache_by_uid_capacity = 0; +static int uid_cache_by_name_capacity = 0; static struct gid_cache_entry *gid_cache = NULL; static int gid_cache_size = 0; @@ -49,13 +51,15 @@ static int gid_cache_capacity = 0; static struct memory_block cache_memory_block = MEMORY_BLOCK_INITIALIZER; -static volatile int cache_rebuild_requested = 1; +static volatile int cache_clear_requested = 1; -static void rebuild_cache(); -static struct uid_cache_entry *uid_cache_lookup(uid_t key); -static struct gid_cache_entry *gid_cache_lookup(gid_t key); +static struct uid_cache_entry *read_through_uid_cache(uid_t uid); +static struct uid_cache_entry *read_through_uid_by_name_cache(const char* name); +static struct gid_cache_entry *read_through_gid_cache(gid_t gid); static int rebuild_uid_cache(); static int rebuild_gid_cache(); +static struct uid_cache_entry* append_to_uid_cache(struct passwd *pw); +static struct gid_cache_entry* append_to_gid_cache(struct group *gr); static void clear_uid_cache(); static void clear_gid_cache(); static int uid_cache_name_sortcmp(const void *key, const void *entry); @@ -65,44 +69,98 @@ static int uid_cache_uid_searchcmp(const void *key, const void *entry); static int gid_cache_gid_sortcmp(const void *key, const void *entry); static int gid_cache_gid_searchcmp(const void *key, const void *entry); -static void rebuild_cache() +static struct uid_cache_entry *read_through_uid_cache(uid_t uid) { - free_memory_block(&cache_memory_block); - init_memory_block(&cache_memory_block, 1024); - rebuild_uid_cache(); - rebuild_gid_cache(); - qsort(uid_cache, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_uid_sortcmp); - qsort(gid_cache, gid_cache_size, sizeof(struct gid_cache_entry), gid_cache_gid_sortcmp); + struct uid_cache_entry *entry = (struct uid_cache_entry *)bsearch( + &uid, + uid_cache_by_uid, + uid_cache_size, + sizeof(struct uid_cache_entry), + uid_cache_uid_searchcmp + ); + if (entry != NULL) { + return entry; + } + + pthread_rwlock_unlock(&cache_lock); + pthread_rwlock_wrlock(&cache_lock); + + struct passwd* pw = getpwuid(uid); + if (pw != NULL) { + entry = append_to_uid_cache(pw); + insertion_sort_last(uid_cache_by_uid, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_uid_sortcmp); + insertion_sort_last(uid_cache_by_name, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_name_sortcmp); + } else { + DPRINTF("Failed to getpwuid(%lld)", (long long)uid); + } + + pthread_rwlock_unlock(&cache_lock); + pthread_rwlock_rdlock(&cache_lock); + return entry; } -static struct uid_cache_entry *uid_cache_lookup(uid_t key) +static struct uid_cache_entry *read_through_uid_by_name_cache(const char* name) { - return (struct uid_cache_entry *)bsearch( - &key, - uid_cache, + struct uid_cache_entry *entry = (struct uid_cache_entry *)bsearch( + name, + uid_cache_by_name, uid_cache_size, sizeof(struct uid_cache_entry), - uid_cache_uid_searchcmp + uid_cache_name_searchcmp ); + if (entry != NULL) { + return entry; + } + + pthread_rwlock_unlock(&cache_lock); + pthread_rwlock_wrlock(&cache_lock); + + struct passwd* pw = getpwnam(name); + if (pw != NULL) { + entry = append_to_uid_cache(pw); + insertion_sort_last(uid_cache_by_uid, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_uid_sortcmp); + insertion_sort_last(uid_cache_by_name, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_name_sortcmp); + } else { + DPRINTF("Failed to getpwnam(%lld)", (long long)uid); + } + + pthread_rwlock_unlock(&cache_lock); + pthread_rwlock_rdlock(&cache_lock); + return entry; } -static struct gid_cache_entry *gid_cache_lookup(gid_t key) +static struct gid_cache_entry *read_through_gid_cache(gid_t gid) { - return (struct gid_cache_entry *)bsearch( - &key, + struct gid_cache_entry *entry = (struct gid_cache_entry *)bsearch( + &gid, gid_cache, gid_cache_size, sizeof(struct gid_cache_entry), gid_cache_gid_searchcmp ); + if (entry != NULL) { + return entry; + } + + pthread_rwlock_unlock(&cache_lock); + pthread_rwlock_wrlock(&cache_lock); + + struct group* gr = getgrgid(gid); + if (gr != NULL) { + entry = append_to_gid_cache(gr); + insertion_sort_last(gid_cache, gid_cache_size, sizeof(struct gid_cache_entry), gid_cache_gid_sortcmp); + } else { + DPRINTF("Failed to getgrgid(%lld)", (long long)gid); + } + + pthread_rwlock_unlock(&cache_lock); + pthread_rwlock_rdlock(&cache_lock); + return entry; } static int rebuild_uid_cache() { - /* We're holding the lock, so we have mutual exclusion on getpwent and getgrent too. */ struct passwd *pw; - struct uid_cache_entry *ent; - int username_len; uid_cache_size = 0; @@ -117,18 +175,12 @@ static int rebuild_uid_cache() } } - if (uid_cache_size == uid_cache_capacity) { - grow_array(&uid_cache, &uid_cache_capacity, sizeof(struct uid_cache_entry)); - } - - ent = &uid_cache[uid_cache_size++]; - ent->uid = pw->pw_uid; - ent->main_gid = pw->pw_gid; - - username_len = strlen(pw->pw_name) + 1; - ent->username_offset = append_to_memory_block(&cache_memory_block, pw->pw_name, username_len); + append_to_uid_cache(pw); } + qsort(uid_cache_by_uid, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_uid_sortcmp); + qsort(uid_cache_by_name, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_name_sortcmp); + endpwent(); return 1; error: @@ -140,16 +192,10 @@ static int rebuild_uid_cache() static int rebuild_gid_cache() { - /* We're holding the lock, so we have mutual exclusion on getpwent and getgrent too. */ struct group *gr; - struct gid_cache_entry *ent; - int i; - struct uid_cache_entry *uid_ent; gid_cache_size = 0; - qsort(uid_cache, uid_cache_size, sizeof(struct uid_cache_entry), uid_cache_name_sortcmp); - while (1) { errno = 0; gr = getgrent(); @@ -161,47 +207,82 @@ static int rebuild_gid_cache() } } - if (gid_cache_size == gid_cache_capacity) { - grow_array(&gid_cache, &gid_cache_capacity, sizeof(struct gid_cache_entry)); - } - - ent = &gid_cache[gid_cache_size++]; - ent->gid = gr->gr_gid; - ent->uid_count = 0; - ent->uids_offset = cache_memory_block.size; - - for (i = 0; gr->gr_mem[i] != NULL; ++i) { - uid_ent = (struct uid_cache_entry *)bsearch( - gr->gr_mem[i], - uid_cache, - uid_cache_size, - sizeof(struct uid_cache_entry), - uid_cache_name_searchcmp - ); - if (uid_ent != NULL) { - grow_memory_block(&cache_memory_block, sizeof(uid_t)); - ((uid_t *)MEMORY_BLOCK_GET(cache_memory_block, ent->uids_offset))[ent->uid_count++] = uid_ent->uid; - } - } + append_to_gid_cache(gr); } + qsort(gid_cache, gid_cache_size, sizeof(struct gid_cache_entry), gid_cache_gid_sortcmp); + endgrent(); return 1; error: endgrent(); clear_gid_cache(); - DPRINTF("Failed to rebuild uid cache"); + DPRINTF("Failed to rebuild gid cache"); return 0; } +static struct uid_cache_entry* append_to_uid_cache(struct passwd *pw) +{ + if (uid_cache_size == uid_cache_by_uid_capacity) { + grow_array(&uid_cache_by_uid, &uid_cache_by_uid_capacity, sizeof(struct uid_cache_entry)); + } + if (uid_cache_size == uid_cache_by_name_capacity) { + grow_array(&uid_cache_by_name, &uid_cache_by_name_capacity, sizeof(struct uid_cache_entry)); + } + + struct uid_cache_entry *entry = &uid_cache_by_uid[uid_cache_size]; + entry->uid = pw->pw_uid; + entry->main_gid = pw->pw_gid; + + int username_len = strlen(pw->pw_name) + 1; + entry->username_offset = append_to_memory_block(&cache_memory_block, pw->pw_name, username_len); + + uid_cache_by_name[uid_cache_size] = *entry; + ++uid_cache_size; + + return entry; +} + +static struct gid_cache_entry* append_to_gid_cache(struct group *gr) +{ + if (gid_cache_size == gid_cache_capacity) { + grow_array(&gid_cache, &gid_cache_capacity, sizeof(struct gid_cache_entry)); + } + + struct gid_cache_entry *entry = &gid_cache[gid_cache_size++]; + entry->gid = gr->gr_gid; + entry->uid_count = 0; + entry->uids_offset = cache_memory_block.size; + + int i; + for (i = 0; gr->gr_mem[i] != NULL; ++i) { + struct uid_cache_entry *uid_entry = read_through_uid_by_name_cache(gr->gr_mem[i]); + if (uid_entry != NULL) { + grow_memory_block(&cache_memory_block, sizeof(uid_t)); + ((uid_t *)MEMORY_BLOCK_GET(cache_memory_block, entry->uids_offset))[entry->uid_count++] = uid_entry->uid; + } + } + + return entry; +} + static void clear_uid_cache() { + free(uid_cache_by_uid); + free(uid_cache_by_name); + uid_cache_by_uid = NULL; + uid_cache_by_name = NULL; uid_cache_size = 0; + uid_cache_by_uid_capacity = 0; + uid_cache_by_name_capacity = 0; } static void clear_gid_cache() { + free(gid_cache); + gid_cache = NULL; gid_cache_size = 0; + gid_cache_capacity = 0; } static int uid_cache_name_sortcmp(const void *a, const void *b) @@ -346,27 +427,28 @@ int user_belongs_to_group(uid_t uid, gid_t gid) pthread_rwlock_rdlock(&cache_lock); - if (cache_rebuild_requested) { + if (cache_clear_requested) { pthread_rwlock_unlock(&cache_lock); pthread_rwlock_wrlock(&cache_lock); - if (cache_rebuild_requested) { - DPRINTF("Building user/group cache"); - cache_rebuild_requested = 0; - rebuild_cache(); + if (cache_clear_requested) { + DPRINTF("Clearing user/group cache"); + cache_clear_requested = 0; + clear_uid_cache(); + clear_gid_cache(); } pthread_rwlock_unlock(&cache_lock); pthread_rwlock_rdlock(&cache_lock); } - struct uid_cache_entry *uent = uid_cache_lookup(uid); - if (uent && uent->main_gid == gid) { + struct uid_cache_entry *uent = read_through_uid_cache(uid); + if (uent != NULL && uent->main_gid == gid) { ret = 1; goto done; } - struct gid_cache_entry *gent = gid_cache_lookup(gid); + struct gid_cache_entry *gent = read_through_gid_cache(gid); if (gent) { uids = (uid_t*)MEMORY_BLOCK_GET(cache_memory_block, gent->uids_offset); for (i = 0; i < gent->uid_count; ++i) { @@ -382,7 +464,33 @@ int user_belongs_to_group(uid_t uid, gid_t gid) return ret; } -void invalidate_user_cache() +void rebuild_user_caches() +{ + pthread_rwlock_wrlock(&cache_lock); + + clear_uid_cache(); + clear_gid_cache(); + free_memory_block(&cache_memory_block); + init_memory_block(&cache_memory_block, 1024); + rebuild_uid_cache(); + rebuild_gid_cache(); + + pthread_rwlock_unlock(&cache_lock); +} + +void invalidate_user_caches() { - cache_rebuild_requested = 1; + cache_clear_requested = 1; +} + +void clear_user_caches() +{ + pthread_rwlock_wrlock(&cache_lock); + + clear_uid_cache(); + clear_gid_cache(); + free_memory_block(&cache_memory_block); + init_memory_block(&cache_memory_block, 1024); + + pthread_rwlock_unlock(&cache_lock); } diff --git a/src/userinfo.h b/src/userinfo.h index 4c0ca55..aa18135 100644 --- a/src/userinfo.h +++ b/src/userinfo.h @@ -35,6 +35,9 @@ int user_uid(const char *username, uid_t *ret); int group_gid(const char *groupname, gid_t *ret); int user_belongs_to_group(uid_t uid, gid_t gid); -void invalidate_user_cache(); /* safe to call from signal handler */ + +void rebuild_user_caches(); +void invalidate_user_caches(); // safe to call from signal handler +void clear_user_caches(); #endif diff --git a/tests/internals/test_internals.c b/tests/internals/test_internals.c index f5dbed5..e7dae4d 100644 --- a/tests/internals/test_internals.c +++ b/tests/internals/test_internals.c @@ -52,7 +52,8 @@ void my_dirname_suite() test_my_dirname(buf, ".."); } -void sprintf_new_suite() { +void sprintf_new_suite() +{ char *result; result = sprintf_new("Hello %d %s", 123, "World"); @@ -64,9 +65,63 @@ void sprintf_new_suite() { free(result); } -void test_internal_suite() { +int compare_int(const void *a, const void *b) +{ + int x = *(int*)a; + int y = *(int*)b; + if (x < y) { + return -1; + } else if (x > y) { + return 1; + } else { + return 0; + } +} + +void test_insertion_sort_last(size_t n, int *elements, int *expected) +{ + insertion_sort_last(elements, n, sizeof(int), &compare_int); + TEST_ASSERT(memcmp(elements, expected, n * sizeof(int)) == 0); +} + +void insertion_sort_last_suite() +{ + { + int elements[6] = { 1, 3, 5, 7, 9, 4 }; + int expected[6] = { 1, 3, 4, 5, 7, 9 }; + test_insertion_sort_last(6, elements, expected); + } + + { + int elements[6] = { 1, 3, 5, 7, 9, 0 }; + int expected[6] = { 0, 1, 3, 5, 7, 9 }; + test_insertion_sort_last(6, elements, expected); + } + + { + int elements[6] = { 1, 3, 5, 7, 9, 10 }; + int expected[6] = { 1, 3, 5, 7, 9, 10 }; + test_insertion_sort_last(6, elements, expected); + } + + { + int elements[6] = { 1, 3, 5, 7, 9, 1 }; + int expected[6] = { 1, 1, 3, 5, 7, 9 }; + test_insertion_sort_last(6, elements, expected); + } + + { + int elements[6] = { 1, 3, 5, 7, 9, 9 }; + int expected[6] = { 1, 3, 5, 7, 9, 9 }; + test_insertion_sort_last(6, elements, expected); + } +} + +void test_internal_suite() +{ my_dirname_suite(); sprintf_new_suite(); + insertion_sort_last_suite(); } TEST_MAIN(test_internal_suite); diff --git a/tests/test_bindfs.rb b/tests/test_bindfs.rb index 33eaaa4..305d385 100755 --- a/tests/test_bindfs.rb +++ b/tests/test_bindfs.rb @@ -780,6 +780,7 @@ def run_chown_chgrp_test_case(chown_flag, chgrp_flag, expectations) `groupdel bindfs_test_group 2>&1` `groupadd -f bindfs_test_group` raise "Failed to create test group" if !$?.success? + testenv("--mirror=@bindfs_test_group", :title => "SIGUSR1 rereads user database") do |bindfs_pid| touch('src/file') chown('nobody', nil, 'src/file') @@ -792,10 +793,30 @@ def run_chown_chgrp_test_case(chown_flag, chgrp_flag, expectations) assert { File.stat('mnt/file').uid == $nobody_uid } Process.kill("SIGUSR1", bindfs_pid) - sleep 0.5 - assert { File.stat('mnt/file').uid == 0 } + assert { wait_for { File.stat('mnt/file').uid == 0 } } + end + + `groupdel bindfs_test_group 2>&1` + `groupadd -f bindfs_test_group` + raise "Failed to create test group" if !$?.success? + + testenv("--mirror=@bindfs_test_group --no-user-group-precaching", :title => "SIGUSR1 rereads user database also in no precaching mode") do |bindfs_pid| + touch('src/file') + chown('nobody', nil, 'src/file') + + assert { File.stat('mnt/file').uid == $nobody_uid } + `usermod -G bindfs_test_group -a root` + raise "Failed to add root to test group" if !$?.success? + + # Cache not refreshed yet + assert { File.stat('mnt/file').uid == $nobody_uid } + + Process.kill("SIGUSR1", bindfs_pid) + + assert { wait_for { File.stat('mnt/file').uid == 0 } } end + ensure `groupdel bindfs_test_group 2>&1` end