| Age | Commit message (Collapse) | Author |
|---|
|
nfsd4_drop_revoked_stid() handles FREE_STATEID for admin-revoked
delegations but does not set SC_STATUS_FREED before releasing cl_lock.
revoke_delegation() uses this flag to detect whether FREE_STATEID has
already processed the delegation -- without it, the freed delegation is
added to cl_revoked via list_add(), producing a use-after-free when
cl_revoked is later traversed in __destroy_client().
The SC_STATUS_REVOKED path in nfsd4_free_stateid() (line 7983) already
sets SC_STATUS_FREED correctly. Apply the same pattern to the
SC_STATUS_ADMIN_REVOKED path in nfsd4_drop_revoked_stid().
Fixes: 8dd91e8d31fe ("nfsd: fix race between laundromat and free_stateid")
Cc: stable@vger.kernel.org
Signed-off-by: Zhenghang Xiao <kipreyyy@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260526104554.46262-1-kipreyyy@gmail.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
->atomic_open is permitted to return success without actually opening
the file. It indicates this by calling finish_no_open().
This means dentry_create() can return a file which hasn't been opened.
This is extremely unlikely as ->atomic_open handlers typically
use finish_no_open() only for already existing files, and dentry_create()
isn't called in that case, and the parent being locked should prevent
races.
However out of an abundance of caution it seems wise to teach nfsd to
only use the file returned by dentry_create() if FMODE_OPENED is set,
indicating that it has in fact been opened.
Fixes: 64a989dbd144 ("VFS/knfsd: Teach dentry_create() to use atomic_open()")
Cc: stable@vger.kernel.org
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260526053004.4014491-3-neilb@ownmail.net
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>
|
|
dentry_create() can hypothetically provide a different dentry than the
one passed in. This could happen, for example, if the exported
filesystem is NFS, and the server returned to OPEN a filehandle which
matched a directory that was already in the dcache. Clearly this would
not be expected!
If this were to happen the dentry (child) that was already stored in
resfhp could be freed and later dereferenced.
We shouldn't call fh_compose() until we are certain that we have the
final dentry, so this patch moved the fh_compose() call to two places:
one for the case where the target already exists, and one after
dentry_create() where it was created.
Fixes: 64a989dbd144 ("VFS/knfsd: Teach dentry_create() to use atomic_open()")
Cc: stable@vger.kernel.org
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260526053004.4014491-2-neilb@ownmail.net
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>
|
|
nfsd4_ssc_expire_umount() walks nn->nfsd_ssc_mount_list with
list_for_each_entry_safe(ni, tmp, ...). For each expired entry it
sets nsui_busy = true, drops nfsd_ssc_lock to run mntput() on the
source vfsmount, then reacquires the lock to list_del + kfree the
entry and continue iterating via the macro's saved tmp pointer.
The nsui_busy flag protects the current ni from concurrent
nfsd4_ssc_setup_dul() finders during the lock-drop window, but it
does not pin tmp. Another nfsd RPC thread that fails its source-
server mount and reaches nfsd4_ssc_cancel_dul() will, during that
same window, take nfsd_ssc_lock, list_del + kfree its own ssc_umount
item, and release the lock. If that item is the saved tmp of the
expire walk, the next iteration dereferences a freed
nfsd4_ssc_umount_item.
Restart the walk from the head after the mntput() unlock window so
no saved next pointer survives the lock-drop. The list is bounded
by the number of active inter-server source mounts (typically small)
and the expire delayed-work runs periodically rather than per-IO,
so the restart is cheap.
Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Link: https://patch.msgid.link/20260524130654.1924556-1-michael.bommarito@gmail.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
nlm_traverse_files() pins the current file with f_count++ across
a mutex_unlock for nlm_inspect_file(), but nothing pins the saved
next pointer. A concurrent nlm_release_file() can kfree the next
file during the unlock window, and the iterator dereferences freed
memory on the next loop step.
Pin both current and next before the lock-drop. Advance by
swapping the pinned cursors at the end of each iteration so next
is always held alive across the unlock.
Always call nlm_file_release() after dropping the iteration pin,
regardless of whether the file matched the predicate. Use
nlm_file_inuse(), which does a live walk of the inode lock list,
rather than the cached f_locks field, so skipped files that never
ran nlm_inspect_file() are evaluated correctly.
Because every file in a hash bucket is now pinned and released,
files skipped by the is_failover_file predicate that have no
locks, blocks, shares, or external references are deleted during
traversal. The old code never evaluated skipped files for
cleanup. The new behavior is intentional: such files are stale
and should not persist in the table.
Fixes: 01df9c5e918a ("LOCKD: Fix a deadlock in nlm_traverse_files()")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Link: https://patch.msgid.link/20260524115527.1734251-1-michael.bommarito@gmail.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
_nfsd_copy_file_range() samples dst->f_wb_err into "since"
after the copy loop, then uses it to detect writeback errors
via filemap_check_wb_err() once vfs_fsync_range() returns.
Because the nfsd_file cache reuses a single struct file
across requests targeting the same inode, a concurrent
COMMIT or stable WRITE on dst advances dst->f_wb_err to the
current mapping->wb_err via file_check_and_advance_wb_err()
during its own vfs_fsync_range(). If that advancement lands
between the writeback error appearing in mapping->wb_err
and the COPY worker sampling "since", the worker captures
the already-advanced cursor, errseq_check() sees cur ==
since and returns zero, and NFSD4_COPY_F_COMMITTED is set
even though writeback failed. CB_OFFLOAD then encodes
wr_stable_how = FILE_SYNC4, the client treats the copied
data as durable, and the failure becomes silent data loss.
Sample since once at the start of the function. The cursor
then reflects state in effect before this COPY issues any
writes, and filemap_check_wb_err() detects any error that
occurs during the copy regardless of which thread first
observes it. This matches the pattern used by
nfsd_vfs_write() and nfsd4_clone_file_range().
Closes: https://sashiko.dev/#/patchset/20260522194441.436065-1-cel@kernel.org?part=1
Fixes: 555dbf1a9aac ("nfsd: Replace use of rwsem with errseq_t")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260522214558.460859-1-cel@kernel.org
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Async COPY captures nn->writeverf at request time and reports it to
the client via CB_OFFLOAD after the worker kthread completes. When
the post-copy vfs_fsync_range() or filemap_check_wb_err() in
_nfsd_copy_file_range() reports an error, the worker correctly
leaves NFSD4_COPY_F_COMMITTED clear so that CB_OFFLOAD encodes
wr_stable_how as NFS_UNSTABLE, but the server's write verifier is
not rotated.
A client that receives NFS_UNSTABLE in CB_OFFLOAD follows up with
COMMIT to make the copied data durable. With the verifier
unchanged, COMMIT returns the same value the client just received
via CB_OFFLOAD, and the client concludes the copy is durable --
silently dropping the data whose writeback in fact failed. This
violates the UNSTABLE+COMMIT durability contract (RFC 7862 section
15.1, RFC 8881 section 18.32) and matches the bug just fixed in
nfsd_vfs_write() and nfsd_commit().
Rotate nn->writeverf at the writeback-failure site. The async COPY
worker has no svc_rqst, so commit_reset_write_verifier() is not
available here; calling nfsd_reset_write_verifier() directly
mirrors the trace-less reset already used by
nfsd_file_check_write_error() for the same purpose. Filter out
-EAGAIN and -ESTALE, matching commit_reset_write_verifier(), since
neither indicates a durable-storage failure.
Fixes: eac0b17a77fb ("NFSD add vfs_fsync after async copy is done")
Cc: stable@vger.kernel.org
Assisted-by: kres:claude-opus-4-7
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260522203723.446841-1-cel@kernel.org
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
When working on a new features that reuses the existing pad in the
superblock, I noticed that mounting such a file system on an old kernel
logs a rather confusing warning:
XFS (vdc): Metadir superblock padding fields must be zero.
This is because we only validate the various feature fields in v5
superblocks after the common superblock validation helper is called.
Fix this by calling the feature validation first.
Fixes: eca383fcd63b ("xfs: refactor superblock verifiers")
Cc: <stable@vger.kernel.org> # v4.19
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
All other fields have comments describing them, add one for this field
as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
Allocating the new bio for a split from the same pool as the original
one can deadlock under memory pressure as the origin bio could be the
last one from the mempool.
Add a separate pool for splitting GC write bios to avoid this.
Fixes: 080d01c41d44 ("xfs: implement zoned garbage collection")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
xlog_recover_dquot_commit_pass2() validates the recovered dquot with
xfs_dqblk_verify() and, on failure, sets error = -EFSCORRUPTED and jumps
to out_release. But out_release unconditionally returns 0, so the
corruption error is discarded: the caller xlog_recover_items_pass2()
sees success, log recovery proceeds as if the dquot were valid, and the
corrupt quota buffer can be written back to disk.
Fixes: 9c235dfc3d3f ("xfs: dquot recovery does not validate the recovered dquot")
Cc: stable@vger.kernel.org # v6.8
Signed-off-by: Long Li <leo.lilong@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
xfs_dq_get_next_id() takes the quota inode ILOCK before calling
xfs_iread_extents(). If xfs_iread_extents() fails, the function returns
immediately without releasing the lock, leaking the quota inode ILOCK.
This can leave the quota inode locked and cause subsequent quota
operations to hang.
Fix this by jumping to a common unlock path on error instead of returning
directly.
Fixes: bda250dbaf39f ("xfs: rewrite xfs_dq_get_next_id using xfs_iext_lookup_extent")
Cc: stable@vger.kernel.org # v4.12
Signed-off-by: Long Li <leo.lilong@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM complained that this function ignores runtime errors being
returned by xrep_iunlink_store_*. Rework the function signature so that
we can return runtime errors to abort the repair.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
If we find a rogue free inode and decide to reinsert it into the
unlinked list, we need to set the prev pointer to NULLAGINO so that the
incore list gets updated.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
xrep_iunlink_resolve_bucket is supposed to reconstruct as much of the
incore prev and next unlinked list pointers based on what it finds on
disk and in memory before we move on to relinking the truly lost inodes
back into the unlinked list. However, it's still vulnerable to infinite
loops that come in via the next_unlinked pointers.
Fix this problem by remembering which inodes we've already seen and
checking new agino pointers against that. If a bit is already set,
either this is a loop or the inode has nonzero link count. We'll deal
with the second case in a subsequent patch.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
If an allocated inode shows up in the unlinked list, we need to get it
completely off the list. Set the corrected next/prev pointers such that
the inode will not look like it should be on an unlinked list at all.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM noticed that we neglect to check for xfarray_iter itself returning
errors when writing a new AGI. Fix that.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM points out that the only error that xrep_iunlink_mark_ondisk_rec
returns is ENOMEM, but we ignore that, and can end up writing a garbage
AGI based on incomplete information. We shouldn't do that, though here
we must be screen out EFSCORRUPTED/EFSBASDCRC because we haven't
checked the inobt yet.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM notices that xrep_iunlink_relink_prev has the comment "set the
forward pointer..." but then loads the value from the xfarray that
stores pointers to the previous inode in the unlinked list. That's
wrong, so fix the variable access.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM noticed a longstanding bug where xrep_iunlink_walk_ondisk_bucket
tries to walk ragi->sc->sa.agi_bp to rebuild the unlinked inode lists.
Unfortunately, it's possible for agi_bp to be null if the buffer
verifier fails, so we have to use ragi->agi_bp (which skips verifier
checks) instead.
Cc: stable@vger.kernel.org # v6.10
Fixes: ab97f4b1c03075 ("xfs: repair AGI unlinked inode bucket lists")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM points out that online fsck can livelock if an unlinked inode list
contains a loop. Use a bitmap to detect cycles.
Cc: stable@vger.kernel.org # v4.15
Fixes: a12890aebb8959 ("xfs: scrub the AGI")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
In the next patch we're going to make this loop more exciting, so hoist
the code to a helper function to reduce clutter in the resulting code.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM noticed a potential UAF if the tempfile creation code fails after
it set sc->tempip. Fix that.
Cc: stable@vger.kernel.org # v6.10
Fixes: 84c14ee39dd388 ("xfs: create temporary files and directories for online repair")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
In xchk_nlinks_ilock_dir, take the IOLOCK before accessing internal
inode state to figure out if we need to take ILOCK shared or exclusive.
That way we can't race with directory updates. LOLLM pointed out that
the code was initially correct w.r.t. the IOLOCK, but then I broke it.
Cc: stable@vger.kernel.org # v6.18
Fixes: f477af0cfa0487 ("xfs: fix locking in xchk_nlinks_collect_dir")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM noticed that xrep_xattr_rebuild_tree doesn't check for queued
parent pointer updates when it decides that it's going to zap the attr
fork. This is obviously incorrect, so fix that. We hold the IOLOCK and
the ILOCK of sc->ip at that point in time, so we can't race with any
/new/ operations.
Cc: stable@vger.kernel.org # v6.10
Fixes: e5d7ce0364d8ee ("xfs: replay unlocked parent pointer updates that accrue during xattr repair")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM noticed that scrub sets the CORRUPT flag when xfs_parent_from_attr
thinks it's been given a corrupt parent pointer. This eliminates the
potential to repair the filesystem because that error code is bubbled up
the call stack. Fix this by collapsing them all to ECANCELED in
xchk_parent_pptr, which doesn't have that trait.
Cc: stable@vger.kernel.org # v6.10
Fixes: 0d29a20fbdba89 ("xfs: scrub parent pointers")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
LOLLM notices that the dirtree scrubber can detect a directory that
refers to itself. In this case, it's not correct for the directory tree
repair code to try to iolock/ilock both sc->ip and dp, because they're
the same inode. Fix this by detecting that corner case and handling it
appropriately.
Cc: stable@vger.kernel.org # v6.10
Fixes: 3f31406aef493b ("xfs: fix corruptions in the directory tree")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
In the cntbt (free space by block count) btree, records are not supposed
to be in startblock order. Hence the mergeability check is pointless.
Remove it, since it does nothing, as LOLLM points out.
Cc: stable@vger.kernel.org # v6.4
Fixes: d5784ae82778d9 ("xfs: flag free space btree records that could be merged")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
sashiko.dev noticed that these checks clear all the valid flags instead
of invalid. This probably was never hit as it only executed on invalid
flag presence.
Fixes: 2d295fe65776 ("xfs: repair inode records")
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
When exchanging two full-file ranges, xmi_can_exchange_reflink_flags()
can move the reflink inode flag from the file that currently has it to
the other file, as long as exactly one side is marked. This assumes
that the file contents, and therefore all shared extents, are exchanged.
That assumption is not true when XFS_EXCHMAPS_INO1_WRITTEN is set.
xfs_exchmaps_can_skip_mapping() can skip hole and unwritten mappings
from file1, so an exchange can complete without moving every mapping
that the earlier flag-swap decision accounted for. In that case the
post-operation cleanup can clear the reflink flag from an inode that
still owns shared written extents. Later writes then take the
non-reflink write path and may update blocks that should still have
been protected by CoW, which shows up as data corruption between
reflink-related files.
Fix this by disabling the reflink flag exchange whenever
XFS_EXCHMAPS_INO1_WRITTEN is requested. The contents exchange can still
proceed; the conservative outcome is that both inodes keep the reflink
flag. The regular reflink flag cleanup path can drop the extra flag
later once the inode no longer has shared extents.
Reported-by: Lin Jiapeng (TencentOS Red Team) <jiapenglin@tencent.com>
Fixes: 966ceafc7a43 ("xfs: create deferred log items for file mapping exchanges")
Cc: stable@vger.kernel.org # v6.10
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Lin Jiapeng <jiapenglin@tencent.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
|
|
An entry is matchable as soon as it is registered. create_entry() sets
the enabled bit for every type and add_entry() links it straight into the
instance, so everything an entry needs has to fit in the write that
creates it.
Add a 'D' flag. The entry is created disabled and has to be enabled by
writing '1' to its entry file before it can match anything. That splits a
registration into create and activate, which a later patch uses to
configure an entry beyond what one register string can carry. It is
useful on its own too. Entries can be staged without dispatching the
moment they are written.
A staged entry stays out of the search list entirely. add_entry() only
hashes an entry that is born matchable, and the first '1' written to
the entry file hashes a staged one, which takes its place in the search
order at that point. The rcu insertion publishes the fully configured
entry, so the exec side keeps the plain enabled test it always had.
Removal cannot rely on the search list anymore. Whether an entry was
already removed is now decided by its dentry, '-1' to the status file
walks the directory instead of the list so staged entries do not
survive it, and a '1' through a file handle held across a removal
publishes nothing.
'D' is consumed at registration and not recorded. What matters afterwards
is whether the entry is enabled, and the entry file already reports that.
A 'B' entry's flags field had to be empty so far because every flag it
could name shaped the invocation, which a bpf handler picks per exec with
bpf_binprm_set_flags(). 'D' shapes the registration instead. So the rule
becomes what it always meant: a 'B' entry carries no invocation flags,
and 'D' composes.
Link: https://patch.msgid.link/20260730-work-binfmt_misc-preopen-v1-1-4a0b0da71f16@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Give bpf handlers the per-exec equivalent of the static 'L' flag. A
load program that sets BPF_BINPRM_LOADER has its selected interpreter
substituted for the binary's PT_INTERP instead of run with the binary
as payload. The binary otherwise executes as a fully native exec.
A single handler can now grade its dispatch per binary: native-arch ELF
with PT_INTERP gets loader substitution for full native identity.
Anything else, such as foreign arch, static, non-ELF can use transparent
or classic dispatch. The load program can read the binary's ELF header
from bprm->buf to make that call.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-19-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Add the first activation of the PT_INTERP substitution machinery. A
static entry registered with the new 'L' flag no longer runs the
registered interpreter with the binary as payload. It stashes the
interpreter as bprm->loader and declines the match with -ENOEXEC. The
format search continues in the same round. binfmt_elf claims the binary
as a fully native exec and substitutes the stashed file for the binary's
PT_INTERP.
'L' rejects every classic-dispatch flag at registration. 'T', 'P' and
'O' have nothing to act on (no argv splice, no execfd) and 'C' is
subsumed (credentials derive from the binary natively). 'F' composes and
is valuable: with it the substitute is pre-opened at registration time
and immune to mount namespace changes. Without it the substitute is
opened at exec time in the exec'ing task's context, so 'L' joins 'C' in
the requirement that the interpreter be named by an absolute path. As
with 'C', only trusted interpreters should be registered. The
substituted loader runs with credentials derived from the binary.
Like the other flag characters 'L' cannot be used as the field
delimiter. The flag scan would run off the registration buffer.
The interpreter open is shared with the classic path via the
entry_open_interpreter() helper. An open error fails the exec. Map
-ENOEXEC to -EACCES to avoid letting the binary run with its own
PT_INTERP.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-18-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Do what binfmt_elf does. When a binfmt_misc loader entry stashed
bprm->loader use it in place of the path named in PT_INTERP, and drop the
override when the binary names no interpreter at all.
Without this 'L' is unusable on nommu, where fdpic is the only ELF
loader. On ARM with an MMU both loaders are registered but split the ELF
space between them along elf_check_fdpic(), so an fdpic binary is never
picked up by binfmt_elf either.
Declining is what fdpic did so far, but it declined late. The pending
override was only caught in begin_new_exec(), by which point the segment
scan had opened the interpreter the binary itself names and overwritten
bprm->buf with its header, leaving the next format in the round to
inspect a buffer that no longer describes the file it is offered. The
scan consumes the override now, so of the in-tree formats only
binfmt_flat still relies on the refusal, and it reads bprm->buf without
writing it.
Transparent dispatch needs nothing on top of the AT_FLAGS translation
both loaders already share. The binary travels in AT_EXECFD, which
create_elf_fdpic_tables() emits, and the exe and comm labelling is done
in exec.c for every format.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-17-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
When a binfmt_misc loader entry stashed bprm->loader use it instead of
opening the path named in PT_INTERP. The substitution deliberately
changes as little as possible. Ownership transfers into the local
interpreter reference which the existing success and error paths already
release.
A binary without PT_INTERP has nothing to substitute for. Drop the
override at the end of the segment scan and load the binary natively.
Nothing sets bprm->loader yet.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-16-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
binfmt_misc currently supports an execution model where the registered
interpreter becomes the executed program and the matched binary is
handed to it as payload. The upcoming binfmt_misc loader mode inverts
this. The matched binary remains the executed program and the registered
interpreter is substituted into the role the binary's PT_INTERP would
have played.
Add the channel for that hand-over. bprm->loader carries an
open_exec-style struct file reference from the binfmt_misc match to the
binary format that consumes it. Unlike bprm->interpreter it does not
request a restart of the format search. The stashing handler declines
the exec with -ENOEXEC and the search continues to the real format in
the same round.
Both ELF loaders consume it, so give them the two helpers to do it with
rather than a copy each. bprm_open_interpreter() hands out the substitute
in place of what PT_INTERP names and bprm_drop_loader() releases one that
turned out not to apply.
Establish the complete lifecycle up front so a stashed loader can
neither leak nor be silently ignored.
- Chain restart: if another format wins the round by staging
bprm->interpreter (binfmt_script) the stashed loader belonged to
the file being replaced. Drop it at the top of the swap block in
exec_binprm().
- Unclaimed or error: free_bprm() releases a still-stashed loader
next to the other bprm file references.
- Silent non-substitution: a final format that reaches
begin_new_exec() with a pending loader would run the binary while
ignoring the override. Refuse with -ENOEXEC before the point of no
return. Formats that do not know about the override (binfmt_flat,
out-of-tree) need no changes.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-15-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Expose transparent mode 'T' to the bpf handler via a new
BPF_BINPRM_TRANSPARENT flag. A bpf handler can decide per binary whether
the dispatch is transparent. This way users may choose a native-looking
loader for one binary and a visible wrapper invocation for the next.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-12-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Let a registration opt into transparent dispatch. The 'T' flag lets a
matched binary keep its argument vector and is sent to the interpreter
through AT_EXECFD. The process's identity is the binary's.
'T' implies 'O' exactly like 'C' does. 'P' is rejected in combination
with it. Transparency preserves the whole argument vector so there is
nothing left for 'P' to say. 'C' remains an independent choice and 'F'
keeps working. A pre-opened interpreter is orthogonal to how the binary
is handed over.
Like the other flag characters 'T' cannot be used as the field
delimiter. The flag scan would run off the registration buffer.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-11-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
A binfmt_misc interpreter is visible to the binary it runs. argv[0]
becomes the interpreter path and the binary's path is appended as an
argument and /proc/pid/cmdline shows both. For wine or qemu-user that is
the point. For a per-binary loader the interpreter is an implementation
detail of running the binary that has no business in the argument
vector. And a binary handed to execveat() as an O_CLOEXEC fd without a
usable path cannot be run through binfmt_misc at all. The interpreter
would have no path to open the binary by.
Add the dispatch machinery for a transparent mode. The binary is handed
to the interpreter through AT_EXECFD. The argument vector is left
exactly as the caller set it. argv[0] and /proc/pid/cmdline look like a
direct execution of the binary. bprm->interp still names the
interpreter: it drives the next format lookup and the
sched_prepare_exec tracepoint, not what the process sees.
The interpreter loads the binary from AT_EXECFD for this. A relocatable
loader can and glibc's ld.so is gaining AT_EXECFD support [1]. A staged
interpreter argument is rejected: no argv slot is built for it to land
in.
The transparent branch raises BINPRM_FLAGS_TRANSPARENT_INTERP. A
dispatch through it labels mm->exe_file with the binary and raises
AT_FLAGS_TRANSPARENT_INTERP next to AT_EXECFD. The aux vector bit is the
loader's hint to retarget saved_auxv and the statistics markers to the
binary, which is only correct while the exe link names the binary too.
The inaccessible-path bail moves after handler selection and into the
path-building branch. A transparent interpreter takes the binary from
AT_EXECFD instead of a path, so the restriction does not apply to it
and the O_CLOEXEC execveat() case above can work. Nothing can take the
transparent branch yet.
Link: https://inbox.sourceware.org/libc-alpha/20260717-work-glibc-binfmt_misc-v3-0-45129bfb13fe@kernel.org [1]
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-10-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
When binfmt_misc dispatches a binary to an interpreter, the interpreter
becomes bprm->file and begin_new_exec() labels mm->exe_file with it. For
wine or qemu-user that is the point. For the transparent mode it
defeats the point. The interpreter is an implementation detail and the
process's identity is the binary. Relocatable programs that locate
themselves via /proc/self/exe find the dynamic linker instead [1].
Userspace cannot get this right on its own. PR_SET_MM_MAP's exe_fd is
gated on checkpoint_restore_ns_capable() in the caller's own user
namespace - that is how CRIU restores an exe link - so the ability to
retarget mm->exe_file is not what this adds. What userspace cannot do
is have the link be right from the first instruction. Credentials are
unaffected either way: they still derive from the interpreter unless
'C' says otherwise.
bprm->executable is the file execve() access-checked and kept open for
AT_EXECFD. It is already the file would_dump() bases the dumpability
decision on and the file bprm->execfd_creds derives credentials from.
Label mm->exe_file with it when the dispatch is transparent and the
identity is correct from the start. The label names precisely the file
the caller passed to execve().
Write-denial moves along with the label. Rather than tracking per mode
who still owes a release, the denial do_open_execat() took stays on
bprm->executable until the file is handed over. begin_new_exec() drops
it right before installing the descriptor - set_mm_exe_file() has taken
its own denial on the identity file by then - and free_bprm() releases
an unconsumed executable with do_close_execat() like the other exec
files. For a transparent dispatch the result is exact parity with a
direct execution: a concurrently written binary fails execve() with
-ETXTBSY at open and a running one cannot be opened for writing. The
interpreter consequently is not exe-pinned and matches the role it has
in a native PT_INTERP exec. A classic execfd dispatch now keeps the
binary write-denied until the exec completes rather than only until the
interpreter swap; the difference is confined to the exec itself.
Nothing sets BINPRM_FLAGS_TRANSPARENT_INTERP yet; the transparent
dispatch machinery in binfmt_misc follows and raises it from birth, so
the label and the aux vector bit that announces it appear together.
Link: https://inbox.sourceware.org/libc-alpha/87ik6fymha.fsf@oldenburg.str.redhat.com [1]
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-9-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
A transparent binfmt_misc dispatch hands the binary to the interpreter
through AT_EXECFD and leaves the argument vector exactly as the caller
built it. The loader on the receiving end has to know which contract it
got.
On the classic 'O'/'C' entries the binary's path is spliced into the
argument vector and the loader consumes arguments. In transparent mode
nothing was spliced and argv belongs entirely to the program. This
cannot be inferred from AT_EXECFD alone. Raise a new AT_FLAGS bit
following the AT_FLAGS_PRESERVE_ARGV0 precedent added for qemu-user in
commit 2347961b11d4 ("binfmt_misc: pass binfmt_misc flags to the
interpreter").
The bit also announces that mm->exe_file names the binary rather than
the interpreter (added in the next commit). A loader that sees the bit
may finish the identity polish by fixing up AT_PHDR/AT_ENTRY/AT_BASE in
saved_auxv and fix the code/data markers via one uncapped PR_SET_MM_MAP
once it has mapped the binary. I've got glibc patches for this as well
but it's useful for any loader.
BINPRM_FLAGS_TRANSPARENT_INTERP carries the mode from binfmt_misc to the
ELF loaders. Both had their own copy of the AT_FLAGS translation, so
give them one bprm_at_flags() to share instead of a second copy that can
drift. Nothing sets the bprm flag yet.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-8-e57866e4ae0f@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
When the format search stages an interpreter exec_binprm() swaps it
in and releases the file it replaces. Dropping the write denial the
open took is done manually ahead of both release paths. The one path
that keeps the file silently relies on it not being called.
Let's just use do_close_execat() on the two paths that release the file
and drop the denial explicitly on the one that does not.
No functional change.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-6-e57866e4ae0f@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Opening the interpreter is a property of the matched entry: an 'F' entry
hands out a clone of the file it pre-opened at registration time, any
other entry opens the selected path. Give that its own helper instead of
an if/else in the middle of load_misc_binary(), and let it fail early
rather than carrying an ERR_PTR through the successful branch.
Building the interpreter's argument vector is the bulk of what remains
and the one part of load_misc_binary() that is specific to the classic
dispatch. Move it into its own helper too, so the dispatch reads as what
it is: pick a handler, pick an interpreter, build the invocation, open
it.
No functional change.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-5-e57866e4ae0f@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
A static entry fixes its invocation flags at registration. A 'B' entry's
load program picks them per exec. Since load_misc_binary() branches on
which kind of entry matched and then applies the two flag sets side by
side every flag is handled twice and each new one has to be added to
both arms.
Translate the 'B' flags into the entry flags they mirror and let the
dispatch act on a single set of flags. The boolean the two arms
communicated 'P' can be removed.
No functional change.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-4-e57866e4ae0f@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Every flag character of the register string is spelled out three
times: in the parser, in the entry's /proc output and in the
delimiter blacklist that keeps a flag character from sending the flag
scan off the end of the buffer. The three lists have to agree, and
each new flag has to be added to all of them.
Describe a flag once - character, entry flag, implied flags and a
description for the registration debug output - and drive all three
from the table.
While at it, express the "a 'B' entry carries no flags" check as what
it is, an empty flags field, rather than as a fourth list of every
flag character. Equivalent: the check runs right after
check_special_flags(), which advances past exactly the flag
characters it consumed and sets exactly their flags.
No functional change.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-3-e57866e4ae0f@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
A 'C' entry computes the credentials from the matched binary instead of
from the interpreter. So a set*id binary hands its credentials to
whatever the entry names as its interpreter.
Without 'F' that interpreter is not opened until the exec happens and
open_exec() resolves the path relative to the current working directory.
The working directory at that point belongs to whoever runs the binary
not to whoever registered the entry. So :x:M::\x7fELF::interp:C lets
every user who execs a matching set*id binary from a directory they
control run their own interp with that binary's credentials.
A relative interpreter has no sensible use here to begin with. The
registering task cannot know what the working directory will be. Make
the register string reject the combination at registration time.
This does refuse register strings that used to be accepted. The 'F' flag
covers the case where the interpreter really is meant to be resolved in
the registrant's context, and it resolves it once, at registration.
Link: https://patch.msgid.link/20260721-work-bpf-binfmt_misc-ptinterp-v2-1-e57866e4ae0f@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
The 'P', 'C' and 'O' flags of a binfmt_misc entry - preserve argv[0],
compute credentials from the binary, and pass the binary as an open file
descriptor - are fixed at registration and apply to every binary the entry
matches. A bpf handler matches, selects the interpreter and reads the
binary per exec, so the flags should be its per-exec decision too: one
handler may match both setuid and non-setuid binaries, argv[0]-sensitive
ones and not.
Honor the flags the load program stages in bprm->bpf_flags through the
bpf_binprm_set_flags() kfunc: BPF_BINPRM_PRESERVE_ARGV0,
BPF_BINPRM_CREDENTIALS and BPF_BINPRM_EXECFD map to 'P', 'C' and 'O' and
keep the semantics of their static counterparts, credentials implying the
open file descriptor included.
Flags staged by a load program that then fails are dropped on the way out
so they cannot leak into a later handler's exec, and the argv[0] decision
acts on the entry's own choice instead of testing the accumulated
bprm->interp_flags bit, which an earlier chain level may have left set and
binfmt_misc never clears.
Since a 'B' entry's flags come from the program, it carries none in the
register string: 'P', 'C' and 'O' are rejected there alongside 'F', which
was already meaningless for it. load_misc_binary() takes the flags from
the entry for a static handler and from bprm->bpf_flags for a bpf one.
Link: https://patch.msgid.link/20260714-work-bpf-binfmt_misc-v2-7-57b7529c002c@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
A bpf binfmt_misc handler selects an interpreter but, unlike binfmt_script,
load_misc_binary() builds the argument vector as just [interpreter, binary,
...] with no slot for an argument to the interpreter. A handler that wants
to reproduce a #! line therefore cannot express its single optional
argument, e.g. a handler that resolves $ORIGIN in a script's #! path loses
the argument that followed the interpreter.
Have load_misc_binary() consume the argument staged through the
bpf_binprm_set_interp_arg() kfunc and insert it between the interpreter and
the binary - the same position and single-argument semantics binfmt_script
gives the argument of a #! line. The argument is cleared once spliced into
the argument vector, and a load program that fails after staging one has it
dropped on the way out: whether the exec fails or -ENOEXEC hands the binary
back to the remaining formats, a stale argument cannot leak into a nested
interpreter's argv. This also lets static-style handlers pass a fixed
interpreter argument, which plain binfmt_misc has never been able to
express.
Link: https://patch.msgid.link/20260714-work-bpf-binfmt_misc-v2-6-57b7529c002c@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
The fs kfuncs are currently exclusive to LSM programs. A binfmt_misc
handler needs a subset of them to do anything interesting: computing an
interpreter relative to the binary's location wants bpf_path_d_path()
on bprm->file->f_path from the load program, and matching on per-binary
metadata wants bpf_get_file_xattr() and friends right from the match
program.
Register the fs kfunc set for struct_ops programs as well and extend
the filter to admit binfmt_misc_ops programs. The xattr setters stay
exclusive to LSM programs: a binary type handler decides how to run a
binary, it has no business modifying filesystem state.
This only takes effect in builds that have the fs kfunc set at all,
i.e. CONFIG_BPF_LSM. Without it a binfmt_misc handler is limited to
bprm fields and the file-backed dynptr, which are provided by the
common kfunc set.
Link: https://lore.kernel.org/20260704211409.1978485-1-farid.m.zakaria@gmail.com
Link: https://patch.msgid.link/20260714-work-bpf-binfmt_misc-v2-5-57b7529c002c@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Activate a registered binfmt_misc_ops handler through the existing text
interface with the new 'B' entry type:
echo ':name:B::::<handler-name>:' > <binfmt_misc>/register
The offset, magic, and mask fields must be empty since the program does
the matching; the interpreter field carries the handler name since the
program supplies the interpreter. Reusing the register file keeps the
existing permission model intact: activating a handler requires the
same write access to a binfmt_misc instance as any other registration,
and the per user namespace instance semantics apply unchanged. A 'B'
entry in a container's own instance shadows the host's handlers just
like any other entry, and the privilege needed to shadow e.g. all ELF
binaries is the same as for a static 'M' entry matching \x7fELF today;
the only novelty is that matching becomes programmable.
The entry takes its own reference on the ops for its whole lifetime.
It is dropped from the SRCU callback that frees the entry rather than
synchronously on the final put: a walker may be asleep inside the
handler's match program while the entry's last reference goes away, so
the ops must stay callable until every walker has left the read
section - the same deferral the entry's own memory already gets. The
registration failure path, where the users refcount is not live yet,
drops it explicitly.
The match program runs from the lookup walk like magic and extension
matching and under the same rules: strict registration order, first
match wins. The walk became an SRCU read-side section in the previous
patch, so the program can sleep: it decides on the actual file
content - program headers beyond the prefetched bprm->buf, say - not
just on whatever happens to be resident in the page cache. A match
commits the exec to the handler. The sleepable load program then
selects the interpreter from load_misc_binary() by calling
bpf_binprm_set_interp() and returning zero; a failure fails the exec
instead of falling through to later entries. The walk is never left
and re-entered, so 'B' entries need no special semantics against
concurrent registration and removal whatsoever. -ENOEXEC keeps its
usual meaning and moves on to the remaining binary formats - a handler
whose load program discovers that it cannot serve the binary after all
hands it back to them - and so does returning zero without having
selected an interpreter; other program-supplied errors are clamped to
the errno range.
The 'F' flag is rejected for 'B' entries: it exists to pre-open a fixed
interpreter at registration time in the registrar's context, and a 'B'
entry has no fixed interpreter to pre-open.
'C' is accepted and behaves exactly as it does for a static entry. It
honors the suid bits of the matched binary while executing the
interpreter, which makes 'B' handlers usable for the setuid case, e.g.
a per-binary loader. This does not let the program's registrant widen
access: bprm_fill_uid() gates the credential transition on
vfsuid_has_mapping() in the caller's user namespace, so the interpreter
can only ever run as a uid that is mapped there, identical to a static
'C' entry. The computed path is opened with open_exec() under the
caller's credentials with the usual LSM and noexec checks, and the
programs run before the transition with the caller's credentials,
never elevated.
Link: https://lore.kernel.org/20260704211409.1978485-1-farid.m.zakaria@gmail.com
Link: https://patch.msgid.link/20260714-work-bpf-binfmt_misc-v2-4-57b7529c002c@kernel.org
Reviewed-by: Farid Zakaria <farid.m.zakaria@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
