binfmt_misc: convert entry list to an hlist

The upcoming conversion of the handler lookup to RCU walks cannot use
list_del_init(): reinitializing the forward pointer of a removed entry
would make a concurrent lockless walker standing on that entry loop
back onto it indefinitely. The removal paths do rely on
reinitialization though because bm_{entry,status}_write() and
bm_evict_inode() need to detect whether an entry has already been
unlinked.

hlists support exactly this pattern: hlist_del_init_rcu() keeps the
forward pointer of the removed entry intact for concurrent walkers and
only zeroes ->pprev with hlist_unhashed() serving as the linked test.

Convert the entry list to an hlist now while keeping the rwlock so the
subsequent RCU conversion is a pure locking change. hlist_add_head()
inserts at the head just as list_add() did so lookup precedence
between registered handlers is unchanged.

Link: https://patch.msgid.link/20260710-work-binfmt_misc-locking-v3-4-a162f7cb58d6@kernel.org
Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
This commit is contained in:
Christian Brauner 2026-07-10 11:33:05 +02:00
parent b8206f516f
commit 7a8b81e8b9
No known key found for this signature in database
GPG Key ID: 91C61BC06578DCA2
3 changed files with 15 additions and 14 deletions

View File

@ -48,7 +48,7 @@ enum {Enabled, Magic};
#define MISC_FMT_OPEN_FILE (1UL << 28)
typedef struct {
struct list_head list;
struct hlist_node node;
unsigned long flags; /* type, status, etc. */
int offset; /* offset of magic */
int size; /* size of magic/mask */
@ -95,7 +95,7 @@ static Node *search_binfmt_handler(struct binfmt_misc *misc,
Node *e;
/* Walk all the registered handlers. */
list_for_each_entry(e, &misc->entries, list) {
hlist_for_each_entry(e, &misc->entries, node) {
char *s;
int j;
@ -665,8 +665,8 @@ static struct binfmt_misc *i_binfmt_misc(struct inode *inode)
*
* If the ->evict call was not caused by a super block shutdown but by a write
* to remove the entry or all entries via bm_{entry,status}_write() the entry
* will have already been removed from the list. We keep the list_empty() check
* to make that explicit.
* will have already been removed from the list. We keep the hlist_unhashed()
* check to make that explicit.
*/
static void bm_evict_inode(struct inode *inode)
{
@ -679,8 +679,8 @@ static void bm_evict_inode(struct inode *inode)
misc = i_binfmt_misc(inode);
write_lock(&misc->entries_lock);
if (!list_empty(&e->list))
list_del_init(&e->list);
if (!hlist_unhashed(&e->node))
hlist_del_init(&e->node);
write_unlock(&misc->entries_lock);
put_binfmt_handler(e);
}
@ -701,7 +701,7 @@ static void bm_evict_inode(struct inode *inode)
static void remove_binfmt_handler(struct binfmt_misc *misc, Node *e)
{
write_lock(&misc->entries_lock);
list_del_init(&e->list);
hlist_del_init(&e->node);
write_unlock(&misc->entries_lock);
locked_recursive_removal(e->dentry, NULL);
}
@ -757,7 +757,7 @@ static ssize_t bm_entry_write(struct file *file, const char __user *buffer,
* read-only. So we only need to take the write lock when we
* actually remove the entry from the list.
*/
if (!list_empty(&e->list))
if (!hlist_unhashed(&e->node))
remove_binfmt_handler(i_binfmt_misc(inode), e);
inode_unlock(inode);
@ -801,7 +801,7 @@ 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);
list_add(&e->list, &misc->entries);
hlist_add_head(&e->node, &misc->entries);
write_unlock(&misc->entries_lock);
simple_done_creating(dentry);
return 0;
@ -874,8 +874,9 @@ static ssize_t bm_status_write(struct file *file, const char __user *buffer,
{
struct binfmt_misc *misc;
int res = parse_command(buffer, count);
Node *e, *next;
struct hlist_node *next;
struct inode *inode;
Node *e;
misc = i_binfmt_misc(file_inode(file));
switch (res) {
@ -901,7 +902,7 @@ static ssize_t bm_status_write(struct file *file, const char __user *buffer,
* read-only. So we only need to take the write lock when we
* actually remove the entry from the list.
*/
list_for_each_entry_safe(e, next, &misc->entries, list)
hlist_for_each_entry_safe(e, next, &misc->entries, node)
remove_binfmt_handler(misc, e);
inode_unlock(inode);
@ -971,7 +972,7 @@ static int bm_fill_super(struct super_block *sb, struct fs_context *fc)
if (!misc)
return -ENOMEM;
INIT_LIST_HEAD(&misc->entries);
INIT_HLIST_HEAD(&misc->entries);
rwlock_init(&misc->entries_lock);
/* Pairs with smp_load_acquire() in load_binfmt_misc(). */

View File

@ -101,7 +101,7 @@ struct linux_binfmt {
#if IS_ENABLED(CONFIG_BINFMT_MISC)
struct binfmt_misc {
struct list_head entries;
struct hlist_head entries;
rwlock_t entries_lock;
bool enabled;
} __randomize_layout;

View File

@ -23,7 +23,7 @@
#if IS_ENABLED(CONFIG_BINFMT_MISC)
struct binfmt_misc init_binfmt_misc = {
.entries = LIST_HEAD_INIT(init_binfmt_misc.entries),
.entries = HLIST_HEAD_INIT,
.enabled = true,
.entries_lock = __RW_LOCK_UNLOCKED(init_binfmt_misc.entries_lock),
};