Linux filesystem development
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: linux-fsdevel@vger.kernel.org
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	 Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	 linux-mm@kvack.org, Farid Zakaria <farid.m.zakaria@gmail.com>,
	 jannh@google.com, stable@vger.kernel.org
Subject: [PATCH v2 00/23] binfmt_misc: write access fixes, RCU handler lookup and cleanups
Date: Fri, 10 Jul 2026 00:29:55 +0200	[thread overview]
Message-ID: <20260710-work-binfmt_misc-locking-v2-0-2a1c3d4126a7@kernel.org> (raw)

The first two patches fix two i_writecount imbalances on
MISC_FMT_OPEN_FILE interpreter files that turned up while auditing
the file for the rework below and are marked for stable: removing an
entry never restored the write access denied by open_exec() at
registration, leaving the interpreter unwritable until its inode gets
evicted, and the write denial taken on the interpreter clone during
exec is not paired with the FMODE_FSNOTIFY_HSM aware release the exec
machinery uses, so pre-content watches make execs leak write denials.

The rest reworks the locking and tidies the file up.

Once binfmt_misc is loaded load_misc_binary() runs for every execve()
on the system since binfmt_misc registers at the head of the formats
list. Every exec therefore performs read_lock() and read_unlock() on
the entries_lock of the relevant binfmt_misc instance, i.e., two
atomic read-modify-writes on a shared cacheline, in the common case
just to conclude that the binary matches no handler. User namespaces
without their own binfmt_misc mount fall back to an ancestor's
instance so on container-heavy systems every exec on the machine
typically ends up hammering the cacheline of init_binfmt_misc. On
PREEMPT_RT the rwlock additionally turns the handler lookup into a
sleeping lock on the exec fast path.

The lock protects very little. Entries are immutable after publication
except for the Enabled bit which is already toggled locklessly via
set_bit()/clear_bit() and entry lifetime is already handled by the
users refcount. The read lock's only remaining job is to make "the
entry is still linked" and "take a reference" atomic with respect to
the unlink sites.

So make the lookup an RCU walk that acquires a reference via
refcount_inc_not_zero() and free entries via kfree_rcu(). The removal
paths need to detect whether an entry has already been unlinked and
rely on list_del_init() reinitialization for that today, but
reinitializing the forward pointer of a removed entry would make a
concurrent lockless walker standing on it loop indefinitely. hlists
support exactly this pattern: hlist_del_init_rcu() keeps the forward
pointer of a removed entry intact for concurrent walkers and only
zeroes ->pprev with hlist_unhashed() serving as the linked test. Hence
the third patch converts the entry list to an hlist so the RCU
conversion in the fourth is a pure locking change.

Writers remain serialized by the inode lock of the root dentry with
one exception: bm_evict_inode() unlinks entries during umount without
holding it. A spinlock stays around the unlink sites to make that
exclusion explicit instead of relying on superblock lifetime rules to
provide it implicitly.

Handler removal semantics are unchanged. An exec that acquired a
reference just before its handler was unregistered already completes
with the removed handler today. The read lock never protected against
that, it only made the window smaller.

With this an exec that matches no binfmt_misc entry no longer writes
to any shared cacheline at all. The fifth patch annotates the
long-standing lockless ->enabled accesses for KCSAN and the three
patches after it make the entry flags proper enums and give struct
binfmt_misc_entry a name that isn't Node.

The remaining patches are a cleanup pass over the whole file: remove
the VERBOSE_STATUS and USE_DEBUG compile-time toggles, convert the
entry file to seq_file, factor out entry matching, entry removal and
the register string field parsing, make the entry/register string
allocation a flexible array member, give the parse_command() results
names, let cleanup.h unwind the entry registration and exec error paths
and prune the include list down to what is used. Aside from
seq_lseek() now bounding seeks on entry files and the ETXTBSY
propagation in the second patch the cleanups have no user-visible
effect.

The last patch adds what the comment in remove_binfmt_handler() had
been suggesting for years: entries can now be removed via unlink(2)
in addition to the -1 write. The status and register control files
refuse removal.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Changes in v2:
- Allow removing entries via unlink(2).
- Add two stable-marked fixes restoring i_writecount balance for
  MISC_FMT_OPEN_FILE interpreter files, first so they apply to
  mainline directly.
- Turn the entry bit numbers and behavior flags into proper enums and
  rename Node to struct binfmt_misc_entry.
- Add a cleanup pass over the whole file: remove the VERBOSE_STATUS
  and USE_DEBUG toggles, convert the entry file to seq_file, factor
  out entry matching, entry removal and register string parsing, use
  a flexible array member for the register string, name the
  parse_command() results, use cleanup.h for the entry error
  unwinding in registration and exec and prune the include list.
- Link to v1: https://patch.msgid.link/20260708-work-binfmt_misc-locking-v1-0-a009dd5b56db@kernel.org

---
Christian Brauner (23):
      binfmt_misc: restore write access when removing an entry
      binfmt_misc: use exe_file_deny_write_access() for the interpreter clone
      binfmt_misc: convert entry list to an hlist
      binfmt_misc: use RCU for the handler lookup
      binfmt_misc: annotate racy accesses to ->enabled
      binfmt_misc: turn the entry bit numbers into a proper enum
      binfmt_misc: turn the entry behavior flags into an enum
      binfmt_misc: rename Node to struct binfmt_misc_entry
      binfmt_misc: remove the VERBOSE_STATUS toggle
      binfmt_misc: use print_hex_dump_debug() for the register debug output
      binfmt_misc: convert the entry file to seq_file
      binfmt_misc: factor out the entry matching
      binfmt_misc: rename load_binfmt_misc() to current_binfmt_misc()
      binfmt_misc: return errors directly in load_misc_binary()
      binfmt_misc: give the parse_command() results names
      binfmt_misc: factor out the entry removal
      binfmt_misc: simplify check_special_flags()
      binfmt_misc: use a flexible array member for the register string
      binfmt_misc: split the field parsing out of create_entry()
      binfmt_misc: use __free(kfree) in bm_register_write()
      binfmt_misc: assorted small cleanups
      binfmt_misc: include what is used
      binfmt_misc: allow removing entries via unlink(2)

 Documentation/admin-guide/binfmt-misc.rst |   3 +-
 fs/binfmt_misc.c                          | 835 +++++++++++++++---------------
 include/linux/binfmts.h                   |   4 +-
 kernel/user.c                             |   4 +-
 4 files changed, 429 insertions(+), 417 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260708-work-binfmt_misc-locking-15347df12ec8


             reply	other threads:[~2026-07-09 22:30 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 22:29 Christian Brauner [this message]
2026-07-09 22:29 ` [PATCH v2 01/23] binfmt_misc: restore write access when removing an entry Christian Brauner
2026-07-09 22:29 ` [PATCH v2 02/23] binfmt_misc: use exe_file_deny_write_access() for the interpreter clone Christian Brauner
2026-07-09 22:29 ` [PATCH v2 03/23] binfmt_misc: convert entry list to an hlist Christian Brauner
2026-07-09 22:29 ` [PATCH v2 04/23] binfmt_misc: use RCU for the handler lookup Christian Brauner
2026-07-10 10:53   ` Jori Koolstra
2026-07-09 22:30 ` [PATCH v2 05/23] binfmt_misc: annotate racy accesses to ->enabled Christian Brauner
2026-07-09 22:30 ` [PATCH v2 06/23] binfmt_misc: turn the entry bit numbers into a proper enum Christian Brauner
2026-07-09 22:30 ` [PATCH v2 07/23] binfmt_misc: turn the entry behavior flags into an enum Christian Brauner
2026-07-09 22:30 ` [PATCH v2 08/23] binfmt_misc: rename Node to struct binfmt_misc_entry Christian Brauner
2026-07-09 22:30 ` [PATCH v2 09/23] binfmt_misc: remove the VERBOSE_STATUS toggle Christian Brauner
2026-07-09 22:30 ` [PATCH v2 10/23] binfmt_misc: use print_hex_dump_debug() for the register debug output Christian Brauner
2026-07-09 22:30 ` [PATCH v2 11/23] binfmt_misc: convert the entry file to seq_file Christian Brauner
2026-07-09 22:30 ` [PATCH v2 12/23] binfmt_misc: factor out the entry matching Christian Brauner
2026-07-09 22:30 ` [PATCH v2 13/23] binfmt_misc: rename load_binfmt_misc() to current_binfmt_misc() Christian Brauner
2026-07-09 22:30 ` [PATCH v2 14/23] binfmt_misc: return errors directly in load_misc_binary() Christian Brauner
2026-07-09 22:30 ` [PATCH v2 15/23] binfmt_misc: give the parse_command() results names Christian Brauner
2026-07-09 22:30 ` [PATCH v2 16/23] binfmt_misc: factor out the entry removal Christian Brauner
2026-07-09 22:30 ` [PATCH v2 17/23] binfmt_misc: simplify check_special_flags() Christian Brauner
2026-07-09 22:30 ` [PATCH v2 18/23] binfmt_misc: use a flexible array member for the register string Christian Brauner
2026-07-09 22:30 ` [PATCH v2 19/23] binfmt_misc: split the field parsing out of create_entry() Christian Brauner
2026-07-09 22:30 ` [PATCH v2 20/23] binfmt_misc: use __free(kfree) in bm_register_write() Christian Brauner
2026-07-09 22:30 ` [PATCH v2 21/23] binfmt_misc: assorted small cleanups Christian Brauner
2026-07-09 22:30 ` [PATCH v2 22/23] binfmt_misc: include what is used Christian Brauner
2026-07-09 22:30 ` [PATCH v2 23/23] binfmt_misc: allow removing entries via unlink(2) Christian Brauner
2026-07-10  7:58 ` [PATCH v2 00/23] binfmt_misc: write access fixes, RCU handler lookup and cleanups Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260710-work-binfmt_misc-locking-v2-0-2a1c3d4126a7@kernel.org \
    --to=brauner@kernel.org \
    --cc=farid.m.zakaria@gmail.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox