diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 86be578787a7..236ebaf3be5c 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -59,6 +60,7 @@ typedef struct { struct dentry *dentry; struct file *interp_file; refcount_t users; /* sync removal with load_misc_binary() */ + struct rcu_head rcu; } Node; static struct file_system_type bm_fs_type; @@ -86,6 +88,8 @@ static struct file_system_type bm_fs_type; * Search for a binary type handler for @bprm in the list of registered binary * type handlers. * + * The caller must hold the RCU read lock. + * * Return: binary type list entry on success, NULL on failure */ static Node *search_binfmt_handler(struct binfmt_misc *misc, @@ -95,7 +99,7 @@ static Node *search_binfmt_handler(struct binfmt_misc *misc, Node *e; /* Walk all the registered handlers. */ - hlist_for_each_entry(e, &misc->entries, node) { + hlist_for_each_entry_rcu(e, &misc->entries, node) { char *s; int j; @@ -134,7 +138,10 @@ static Node *search_binfmt_handler(struct binfmt_misc *misc, * @bprm: binary for which we are looking for a handler * * Try to find a binfmt handler for the binary type. If one is found take a - * reference to protect against removal via bm_{entry,status}_write(). + * reference to protect against removal via bm_{entry,status}_write(). The + * refcount of an entry can only drop to zero once it has been unlinked and + * a restarted search cannot find an unlinked entry again so the retry loop + * is bounded. * * Return: binary type list entry on success, NULL on failure */ @@ -143,11 +150,10 @@ static Node *get_binfmt_handler(struct binfmt_misc *misc, { Node *e; - read_lock(&misc->entries_lock); - e = search_binfmt_handler(misc, bprm); - if (e) - refcount_inc(&e->users); - read_unlock(&misc->entries_lock); + guard(rcu)(); + do { + e = search_binfmt_handler(misc, bprm); + } while (e && !refcount_inc_not_zero(&e->users)); return e; } @@ -166,7 +172,8 @@ static void put_binfmt_handler(Node *e) exe_file_allow_write_access(e->interp_file); filp_close(e->interp_file, NULL); } - kfree(e); + /* Lockless walkers may still dereference this entry. */ + kfree_rcu(e, rcu); } } @@ -678,10 +685,10 @@ static void bm_evict_inode(struct inode *inode) struct binfmt_misc *misc; misc = i_binfmt_misc(inode); - write_lock(&misc->entries_lock); + spin_lock(&misc->entries_lock); if (!hlist_unhashed(&e->node)) - hlist_del_init(&e->node); - write_unlock(&misc->entries_lock); + hlist_del_init_rcu(&e->node); + spin_unlock(&misc->entries_lock); put_binfmt_handler(e); } } @@ -700,9 +707,9 @@ static void bm_evict_inode(struct inode *inode) */ static void remove_binfmt_handler(struct binfmt_misc *misc, Node *e) { - write_lock(&misc->entries_lock); - hlist_del_init(&e->node); - write_unlock(&misc->entries_lock); + spin_lock(&misc->entries_lock); + hlist_del_init_rcu(&e->node); + spin_unlock(&misc->entries_lock); locked_recursive_removal(e->dentry, NULL); } @@ -753,9 +760,11 @@ static ssize_t bm_entry_write(struct file *file, const char __user *buffer, * via bm_{entry,register,status}_write() inode_lock() on the * root inode must be held. * The lock is exclusive ensuring that the list can't be - * modified. Only load_misc_binary() can access but does so - * read-only. So we only need to take the write lock when we - * actually remove the entry from the list. + * modified. Only load_misc_binary() can access the list + * concurrently and it does so under RCU. So entries_lock only + * needs to be held when an entry is actually unlinked to + * serialize against bm_evict_inode() during umount which + * unlinks without holding inode_lock. */ if (!hlist_unhashed(&e->node)) remove_binfmt_handler(i_binfmt_misc(inode), e); @@ -800,9 +809,9 @@ static int add_entry(Node *e, struct super_block *sb) d_make_persistent(dentry, inode); misc = i_binfmt_misc(inode); - write_lock(&misc->entries_lock); - hlist_add_head(&e->node, &misc->entries); - write_unlock(&misc->entries_lock); + spin_lock(&misc->entries_lock); + hlist_add_head_rcu(&e->node, &misc->entries); + spin_unlock(&misc->entries_lock); simple_done_creating(dentry); return 0; } @@ -898,9 +907,11 @@ static ssize_t bm_status_write(struct file *file, const char __user *buffer, * via bm_{entry,register,status}_write() inode_lock() on the * root inode must be held. * The lock is exclusive ensuring that the list can't be - * modified. Only load_misc_binary() can access but does so - * read-only. So we only need to take the write lock when we - * actually remove the entry from the list. + * modified. Only load_misc_binary() can access the list + * concurrently and it does so under RCU. So entries_lock only + * needs to be held when an entry is actually unlinked to + * serialize against bm_evict_inode() during umount which + * unlinks without holding inode_lock. */ hlist_for_each_entry_safe(e, next, &misc->entries, node) remove_binfmt_handler(misc, e); @@ -973,7 +984,7 @@ static int bm_fill_super(struct super_block *sb, struct fs_context *fc) return -ENOMEM; INIT_HLIST_HEAD(&misc->entries); - rwlock_init(&misc->entries_lock); + spin_lock_init(&misc->entries_lock); /* Pairs with smp_load_acquire() in load_binfmt_misc(). */ smp_store_release(&user_ns->binfmt_misc, misc); diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 071da63f2b48..7e7333b7bb0f 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -102,7 +102,7 @@ struct linux_binfmt { #if IS_ENABLED(CONFIG_BINFMT_MISC) struct binfmt_misc { struct hlist_head entries; - rwlock_t entries_lock; + spinlock_t entries_lock; bool enabled; } __randomize_layout; diff --git a/kernel/user.c b/kernel/user.c index c6a2bfb4d918..21bafdc11379 100644 --- a/kernel/user.c +++ b/kernel/user.c @@ -25,7 +25,7 @@ struct binfmt_misc init_binfmt_misc = { .entries = HLIST_HEAD_INIT, .enabled = true, - .entries_lock = __RW_LOCK_UNLOCKED(init_binfmt_misc.entries_lock), + .entries_lock = __SPIN_LOCK_UNLOCKED(init_binfmt_misc.entries_lock), }; EXPORT_SYMBOL_GPL(init_binfmt_misc); #endif