mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/
synced 2026-11-10 20:48:12 +00:00
1,482,263 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Christian Brauner
|
cbade031e2 |
Merge patch series "mount: more bugfixes, the Oprah edition"
Christian Brauner <brauner@kernel.org> says: Yet more bugfixes falling out of my recent work in this area: - don't let a pseudo dentry become the root of a mount - don't drop active namespace references that were never taken - don't put a mountpoint on a dentry that's being removed - remove the fsnotify marks of a mount namespace in process context - don't reconfigure internal superblocks via remount and umount - check the mounts before reading their parents in pivot_root() - look at the topmost mount for a mount namespace file - keep covered mounts covered in OPEN_TREE_NAMESPACE - check a recursive bind mount for mount namespace loops - check a submount for references right before unmounting it - queue a mount only once for mount notifications * patches from https://patch.msgid.link/20260930-work-mount-fixes-3-v1-0-be34c83956ae@kernel.org: namespace: don't let a pseudo dentry become the root of a mount unshare: don't drop active namespace references that were never taken dcache: don't put a mountpoint on a dentry that's being removed namespace: remove the fsnotify marks of a mount namespace in process context selftests/filesystems: check that the nullfs root can't be reconfigured namespace: don't reconfigure internal superblocks via remount and umount namespace: check the mounts before reading their parents in pivot_root() selftests/filesystems: check that a mount namespace file on top doesn't bury a mount namespace: look at the topmost mount for a mount namespace file selftests/filesystems: check that OPEN_TREE_NAMESPACE keeps mounts covered namespace: keep covered mounts covered in OPEN_TREE_NAMESPACE selftests/filesystems: check that a recursive bind mount can't pin the caller's mount namespace namespace: check a recursive bind mount for mount namespace loops selftests/filesystems: check that a busy submount survives a synchronous umount namespace: check a submount for references right before unmounting it namespace: queue a mount only once for mount notifications Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-0-be34c83956ae@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
b138e16fb1 |
namespace: don't let a pseudo dentry become the root of a mount
d_alloc_pseudo() hands out dentries for pipes, sockets and other files
that are never anyone's child or parent. They carry DCACHE_NORCU and
dentry_free() frees them right away because no lockless path walk can
ever reach them. That holds as long as such a dentry isn't the root of
a mount. But bind mounting /proc/self/fd/<fd> of such a file does
exactly that.
Most callers of alloc_file_pseudo() put their files on kernel-internal
mounts and may_copy_tree() refuses those. bpf_token_create() doesn't. It
places the token file on the bpffs mount the caller handed it and that
mount is in the caller's mount namespace so the clone goes through:
mount --bind /proc/self/fd/<token> <file>
open_tree(tokfd, "", AT_EMPTY_PATH | OPEN_TREE_CLONE) + move_mount()
__follow_mount_rcu() then loads ->mnt_root of that mount, reads d_seq
and d_flags of the dentry and only then checks mount_lock. The dentry
can be gone by then. The final mntput() of an unmounted parent unhooks
a child that stayed attached to it under mount_lock alone. When the
child's holder does the final put right after that the root is dput()
and freed immediately while a walker that found the mount hashed still
looks at it.
Refuse to clone a mount with a DCACHE_NORCU dentry as its root. Nothing
sensible can be done with a bind mount of a bpf token anyway.
Fixes:
|
||
|
Christian Brauner
|
0b6f31a3c0 |
unshare: don't drop active namespace references that were never taken
Active references on the namespaces of an nsproxy are taken when the nsproxy is installed into a task in switch_task_namespaces() and copy_namespaces() and dropped again by put_nsproxy() through deactivate_nsproxy(). But ksys_unshare() calls put_nsproxy() on an nsproxy that was never installed when set_cred_ucounts() fails. The new namespaces of that nsproxy go from zero to minus one and the namespaces shared with the caller lose a reference that belongs to the nsproxy the caller keeps using: WARNING: kernel/nscommon.c:171 at __ns_ref_active_put+0x1cd/0x230 nsproxy_ns_active_put deactivate_nsproxy ksys_unshare Afterwards the namespaces the caller lives in aren't listed by listns() anymore. set_cred_ucounts() only fails when alloc_ucounts() can't allocate and it only allocates when the real uid of the caller differs from its effective uid. Commit |
||
|
Christian Brauner
|
75f4197f1e |
dcache: don't put a mountpoint on a dentry that's being removed
rmdir(), unlink() and rename() call dont_mount() on the victim and then
detach_mounts() with the victim's inode locked. do_lock_mount() takes
the inode lock of the mountpoint and checks cant_mount() so a mount
can't show up after detach_mounts().
But attach_recursive_mnt() makes a second mountpoint for the root of the
source mount so that the mounts already located at the destination can
be put on top of it. No inode is locked for that one and d_set_mounted()
only refuses a dentry that is unlinked. Between detach_mounts() and
d_delete() the victim is still hashed:
rmrace: b passed dont_mount() and detach_mounts(), sleeping
T2: move_mount(S, /tmp/plcant/x, BENEATH) = 0 errno 0 ()
T1: rmdir(/tmp/plcant/d/b) = 0 errno 0
109 107 0:61 /d/b//deleted /tmp/plcant/x rw,relatime - tmpfs tmpfs rw
108 109 0:63 / /tmp/plcant/x rw,relatime - tmpfs T rw
A bind mount of the directory that's being removed is moved beneath an
existing mount while the rmdir() is located between the two calls. The
mount ends up on the removed directory and nothing will ever detach it.
Check cant_mount() in d_set_mounted() as well. dont_mount() raises the
flag under d_lock before detach_mounts() runs so either the mountpoint
is set first and detach_mounts() finds the mount or the flag is seen and
the mount is refused.
Fixes:
|
||
|
Christian Brauner
|
1a116222ac |
namespace: remove the fsnotify marks of a mount namespace in process context
mnt_ns_release() drops the last passive reference of a mount namespace
and removes its fanotify marks via fsnotify_mntns_delete(). That takes
the mutex of every group with a mark on the namespace and the spinlock
of the connector. Fine from process context. But mnt_ns_tree_remove()
hands the reference the namespace was allocated with to call_rcu() and
so the marks are removed from the RCU softirq:
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:623
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0, name: swapper/3
__mutex_lock+0x113/0x24b0
fsnotify_destroy_marks+0x11b/0x3d0
mnt_ns_release_rcu+0x57/0xa0
rcu_core+0x6b6/0x1e40
and lockdep complains about the connector lock being taken from softirq
context. A fanotify group with a FAN_MARK_MNTNS mark on the mount
namespace of another task is all that's needed. Root in a user
namespace can do that for a mount namespace it owns. The task exits and
the namespace is freed with the mark still on it.
Remove the marks in free_mnt_ns() before the namespace is handed to RCU.
That runs in process context once the last active reference is gone. A
mark is added through a file descriptor to the namespace which holds an
active reference so no mark can show up after that.
Fixes:
|
||
|
Christian Brauner
|
5e83e3a38b |
selftests/filesystems: check that the nullfs root can't be reconfigured
Add a test for the root of an empty mount namespace: - fspick() of the root fails with EINVAL - mount(MS_REMOUNT) of the root fails with EINVAL - umount() of the root fails and doesn't remount it read-only Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-12-be34c83956ae@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
d791606bb9 |
namespace: don't reconfigure internal superblocks via remount and umount
Commit |
||
|
Christian Brauner
|
6b7c001eb5 |
namespace: check the mounts before reading their parents in pivot_root()
path_pivot_root() reads the parents of new_root and of the caller's root
and checks whether they are shared before it checks that either mount is
in the caller's mount namespace. Only namespace_sem is held. That's fine
for a mount that is in the caller's mount namespace.
But new_root can be a file descriptor to a mount that has been unmounted
and that only the file descriptor keeps alive. If that mount stayed
attached to its parent when it was unmounted nothing pins the parent for
it. The final mntput() of the parent unhooks the children under
mount_lock alone and frees the parent afterwards:
pivot_root() close(fd), last ref on the parent
------------ ---------------------------------
ex_parent = new_mnt->mnt_parent
mntput_no_expire_slowpath()
__umount_mnt(new_mnt)
cleanup_mnt()
call_rcu()
IS_MNT_SHARED(ex_parent)
BUG: KASAN: slab-use-after-free in path_pivot_root+0xf1a/0x1840
Read of size 4 at addr ffff8881047382f8 by task pivot_widen/157
path_pivot_root+0xf1a/0x1840
__x64_sys_pivot_root+0x165/0x190
The same goes for the caller's root via chroot(). Both outcomes of the
check end in EINVAL so nothing but the read itself goes wrong. It's the
same thing commit
|
||
|
Christian Brauner
|
c518a195f6 |
selftests/filesystems: check that a mount namespace file on top doesn't bury a mount
Add a test for moving a mount with a mount namespace file on top of it: - S, a bind mount of a file, with a newer mount namespace's file on top - A shared with a slave B that has Q on B/file - S moved onto A/file, its copy lands on B/file below Q B/file keeps reading Q and once Q is unmounted it reads the copy. Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-9-be34c83956ae@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
8714b44ada |
namespace: look at the topmost mount for a mount namespace file
attach_recursive_mnt() preallocates a mountpoint for the root of the
topmost mount of the source so that a mount that already sits at the
destination can be put on top of the source or on top of one of its
propagated copies.
The copies are made without CL_COPY_MNT_NS_FILE so their chain of
overmounts is shorter than the source's. It ends below the first bind
mount of a mount namespace file. So while the loop walks up the chain of
the source it remembers the mountpoint of that mount in "shorter" and
the existing mount is put there for the copies.
But the loop ends at the topmost mount without ever looking at it. If
the topmost mount is the only mount namespace file in the chain
"shorter" stays NULL and mnt_change_mountpoint() attaches the existing
mount of the copy below the root of the mount namespace file. That's a
dentry of nsfs. No path walk in the copy's mount namespace ever gets
there. The mount is gone until its parent goes and the copy that took
its place can't be unmounted synchronously because it has a child:
S bind mount of a file
N bind mount of a mount namespace file on top of S
A shared mount, B a slave of A, Q mounted on B/file
move_mount(S, A/file)
cat B/file -> the content of S instead of Q
umount B/file -> EBUSY
Look at every mount of the chain including the topmost one.
Fixes:
|
||
|
Christian Brauner
|
fe187cbb48 |
selftests/filesystems: check that OPEN_TREE_NAMESPACE keeps mounts covered
Add a test for open_tree(OPEN_TREE_NAMESPACE) from a user namespace that doesn't own the mount namespace it copies from: - a file covered by a private or an unbindable tmpfs mount - the non-recursive copy of the parent mount fails with EINVAL - the recursive copy keeps the file covered in the new mount namespace - the owner of the mount namespace keeps the bind mount semantics Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-7-be34c83956ae@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
0febe278ec |
namespace: keep covered mounts covered in OPEN_TREE_NAMESPACE
open_tree(OPEN_TREE_NAMESPACE) creates a new mount namespace from a
copy of the tree at the given path. The caller needs to be privileged
over its current user namespace because the new mount namespace will be
owned by it. It doesn't need to be privileged over the mount namespace
the tree is copied from. An unprivileged user just needs to create a
user namespace first.
The copy follows the rules of a detached bind mount. Without
AT_RECURSIVE only the mount itself is cloned and its children are left
out. With AT_RECURSIVE unbindable mounts are skipped. Both is fine when
the caller has privileges over the source mount namespace because it
could unmount those mounts anyway. Not so for an unprivileged user in
that mount namespace. For them a mount namespace copy is what unshare()
does. copy_mnt_ns() copies everything including unbindable mounts and
lock_mnt_tree() makes sure nothing can be unmounted in the copy. Nothing
that was covered gets revealed.
create_new_namespace() only does the locking and so the covered content
is right there in the new mount namespace:
# mount -t tmpfs none /tmp/otn; mkdir /tmp/otn/covered
# echo hidden > /tmp/otn/covered/under.txt
# mount -t tmpfs none /tmp/otn/covered
uid 1000, after unshare(CLONE_NEWUSER):
fd = open_tree(AT_FDCWD, "/tmp/otn", OPEN_TREE_NAMESPACE);
setns(fd, CLONE_NEWNS);
open("/covered/under.txt", O_RDONLY) -> "hidden"
Covering paths with mounts is how container runtimes mask parts of
/proc and /sys and how admins hide things.
When the caller's user namespace doesn't own the source mount namespace
copy the way unshare() does and refuse a non-recursive copy of a mount
that has anything mounted below the requested directory and copy
unbindable mounts in a recursive copy. A caller that is privileged over
the source mount namespace sees no change.
Fixes:
|
||
|
Christian Brauner
|
7c07b18300 |
selftests/filesystems: check that a recursive bind mount can't pin the caller's mount namespace
Add a test for a recursive bind mount of a namespace file in another mount namespace: - a bind mount of the network namespace file, held through a descriptor - the caller's own mount namespace file stacked on top of it there - the recursive bind mount through the descriptor fails with EINVAL - a plain bind mount of the file still works The copy would pin the namespace it is put in otherwise. Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-5-be34c83956ae@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
c640a01a78 |
namespace: check a recursive bind mount for mount namespace loops
do_loopback() refuses to bind mount the file of a mount namespace that is as old as the caller's or older because a mount namespace that holds a mount of its own file, or of an ancestor's, would cause a cycle. But it only checks the dentry the bind mount starts from. With MS_REC everything below it is copied and do_loopback() passes CL_COPY_MNT_NS_FILE so mount namespace files below the source are copied. That's fine for a source in the caller's own mount namespace. Every mount namespace file in there passed the same check when it was bind-mounted. It isn't fine for a source in another mount namespace. may_copy_tree() accepts a bind mount of any nsfs or pidfs file no matter what mount namespace it lives in so that /proc/<pid>/ns/<ns> and pidfds can be bind mounted. If we create a bind-mount stack of mount namespaces file descriptors the kernel will copy them irrespective of their ancestoral relationship to the mount namespace in question: 151 146 0:7 net:[4026531833] /tmp/nrc/y rw - nsfs nsfs rw 152 151 0:7 mnt:[4026532293] /tmp/nrc/y rw - nsfs nsfs rw Mount 152 is a mount of the mount namespace file of mount namespace 4026532293 inside mount namespace 4026532293. That namespace and every mount in it are leaked... All it takes is a process in an older mount namespace that stacks the file and repeating it leaks without limit: after control: Shmem: 380 kB child: mount(/proc/self/fd/6, MS_BIND|MS_REC): ok after cycle: Shmem: 65916 kB do_move_mount() runs check_for_nsfs_mounts() over a detached tree for exactly that reason. Do the same for the copy before it is grafted. Fixes: |
||
|
Christian Brauner
|
520b5a15fc |
selftests/filesystems: check that a busy submount survives a synchronous umount
Add a test for the shrinkable submounts of a synchronous umount: - P shared, P1 a slave that is shared in turn, P2 its peer - B on P/options with copies on P1 and P2, R on top of the copy in P2 - P with B moved below P1, R is the working directory - umount(P1) slides R to where the copy of B is looked up The umount fails with EBUSY and R stays mounted. Needs the tracefs automount below debugfs for the shrinkable mounts. Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-3-be34c83956ae@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
32bf924166 |
namespace: check a submount for references right before unmounting it
shrink_submounts() and mark_mounts_for_expiry() first collect all the
mounts they are allowed to unmount and then unmount them.
Whether a mount is busy is decided by propagate_mount_busy() on the
tree. But unmounting the first mount changes the tree that the second
one propagates into. propagate_umount()
moves a surviving overmount off a copy it unmounts and mounts it where
the copy was mounted. If that's where the propagated copy of the second
victim is looked up the overmount becomes a candidate of the second
umount_tree(). It's childless and so trim_one() commits it without
looking at its reference count.
So a synchronous umount pulls out a busy mount that is in use somewhere
else and marks it MNT_SYNC_UMOUNT while it has users:
umount2(/tmp/plshrink/p1, 0) = 0 errno 0
cwd is (unreachable)
The same two umounts requested one after the other fail with EBUSY.
This needs a mount with MNT_SHRINKABLE, i.e., automounted submounts of
NFS, AFS and CIFS or the tracefs mount below debugfs. Bind mounts
inherit the flag.
Check each mount right before it is unmounted under the same hold of
namespace_sem and mount_lock as the umount itself. Make sure that the
algorithm stays linear.
Fixes:
|
||
|
Christian Brauner
|
bd2feb7796 |
namespace: queue a mount only once for mount notifications
mnt_notify_add() puts a mount on notify_list. It doesn't check whether
the mount is on the list already. That's fine as long as every mount is
queued at most once per namespace_sem hold. It isn't.
propagate_umount() moves a surviving overmount off a stack of mounts
that are going away and queues it as moved. shrink_submounts() and
mark_mounts_for_expiry() call umount_tree() for several mounts under a
single namespace_sem hold. So the second umount_tree() can take down
exactly the mount the first one reparented, reparent it once more and so
end up queueing it a second time.
The second list_add_tail() cuts the mounts queued in between out of the
list while the head still points at the last of them. notify_mnt_list()
then only visits that one mount and the others are freed after the grace
period with notify_list still pointing at them. Every later mount
operation on the host walks freed memory:
BUG: KASAN: slab-use-after-free in __list_add_valid_or_report
Read of size 8 at addr ffff8881003d57e0 by task notify_dq/162
__list_add_valid_or_report+0x15c/0x1a0
mnt_add_to_ns+0x322/0x890
attach_recursive_mnt.isra.0+0xf2a/0x1a00
path_mount+0x139c/0x1d40
This needs a mount with MNT_SHRINKABLE to bind from. That's what
finish_automount() creates (submounts of NFS, AFS and CIFS and the
tracefs mount below debugfs). Bind mounts inherit the flag. A mount
namespace with a FAN_MARK_MNTNS mark does the rest and root in a user
namespace can have both.
Initialize to_notify in alloc_vfsmnt() and leave a mount alone that is
queued already. mnt_notify() looks at the state the mount has when the
list is drained so a single entry per mount is enough. The move event
for a mount that is unmounted under the same hold is lost. That's fine.
The detach is what matters.
Fixes:
|
||
|
Christian Brauner
|
b4698e50d4 |
Merge patch series "mount: a few additional bugfixes"
Christian Brauner <brauner@kernel.org> says: Here's a set of additional mount fixes that fell out of my recent look at the mount code: - Fix a UAF in mnt_change_mountpoint(). - Fix a UAF in statmount(). - FIX a UAF and various associated bugs in clone_mnt() - Prevent copying an unmounted mount tree. - Avoid pointless work in mount_setattr(). - Refuse fspick() on kernel internal superblocks. - Simplify dissolve_on_fput() and unshare(). * patches from https://patch.msgid.link/20260926-work-mount-fixes-2-v1-0-f357abf3d17b@kernel.org: selftests/filesystems: check that a dead mount's overmount isn't followed selftests/filesystems: check that an unmounted mount tree isn't walked namespace: drop the file's reference first in dissolve_on_fput() fs: put the old fs_struct before the old namespaces in unshare() fs: refuse fspick() on internal superblocks namespace: check the target before walking it in do_mount_setattr() namespace: don't copy an unmounted mount tree namespace: don't inherit MNT_UMOUNT in clone_mnt() statmount: read the parent of an unmounted mount under mount_lock namespace: reset the old parent's ->overmount in mnt_change_mountpoint() Link: https://patch.msgid.link/20260926-work-mount-fixes-2-v1-0-f357abf3d17b@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
7fbcc793f7 |
selftests/filesystems: check that a dead mount's overmount isn't followed
Add overmount_reparent_test: - a mount that a file descriptor keeps alive after propagate_umount() reparented its overmount and unmounted it must not be walked into the freed overmount by move_mount(MOVE_MOUNT_BENEATH) Link: https://patch.msgid.link/20260926-work-mount-fixes-2-v1-10-f357abf3d17b@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
07617913d0 |
selftests/filesystems: check that an unmounted mount tree isn't walked
Add unmounted_tree_test: - a detached nsfs or pidfs bind mount that a file descriptor keeps alive can't be copied recursively, a copy of the mount itself and a copy of a mounted one still work - a detached tree with a child that was vacated behind it can't be copied recursively either - a copy of a detached bind mount is an ordinary mount: unlinking its mountpoint from another mount namespace unmounts it - mount_setattr() refuses to change the propagation of a detached tree and still allows it for the root of an anonymous mount namespace Every case runs in a user and mount namespace of its own. Link: https://patch.msgid.link/20260926-work-mount-fixes-2-v1-9-f357abf3d17b@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
7253a6ff64 |
namespace: drop the file's reference first in dissolve_on_fput()
A detached mount tree is held alive by a file. dissolve_on_fput() unmounts the detached tree attached to that file. Currently dissolve_on_fput() leaves the file's reference to the root of the tree to the caller. So either __fput(), open_detached_copy(), or fsmount() drop the mount. That's backwards. The reference keeps the tree around and is dropped when the tree is already gone. Instead, let dissolve_on_fput() the mount and drop it after umount_tree() with namespace_sem still held. After umount_tree() the mntput() is never the last reference and so the root gets unmounted before its children. Link: https://patch.msgid.link/20260926-work-mount-fixes-2-v1-8-f357abf3d17b@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
57088a37b2 |
fs: put the old fs_struct before the old namespaces in unshare()
unshare(CLONE_NEWNS) always copies the fs_struct and copy_mnt_ns() points root and pwd of that copy into the new mount namespace. The old fs_struct pins the old root and pwd until ksys_unshare() frees it at the end and switch_task_namespaces() will already put the old mount namespace before. This causes pointless work in user namespaces because the copied mounts are locked and stay connected. Switch and free the fs_struct before switching the namespaces. The old root and pwd are wasted while their mount namespace is still alive which also keeps both puts on the mntput() fast path. setns() already does this right as it updates root and pwd before the namespaces are switched. Link: https://patch.msgid.link/20260926-work-mount-fixes-2-v1-7-f357abf3d17b@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
02587a4af8 |
fs: refuse fspick() on internal superblocks
There's a few instances where a internal superblock is reachable by
fspick(). Block that. graft_tree() and fsmount() already refuse them.
fanotify refuses mount and superblock marks on them for the same reason.
Fixes:
|
||
|
Christian Brauner
|
08f4288a37 |
namespace: check the target before walking it in do_mount_setattr()
When mount_setattr() changes the propagation type to MS_SHARED
do_mount_setattr() takes namespace_sem and calls invent_group_ids()
which walks the whole target tree with next_mnt() and allocates a peer
group id for every mount that doesn't have one.
Whether the caller may touch the target at all is only checked
afterwards under mount_lock. If it may not, cleanup_group_ids() walks
the tree a second time on the way out to free the ids again.
A detached tree that a file descriptor keeps alive, a mount in another
mount namespace or a mount below the root of an anonymous mount
namespace does a bunch of allocation work that is completely useless
because it will fail and revert right after.
Do the right thing and check the target right after taking namespace_sem
instead. A mount leaves its namespace only under namespace_sem so the
mount stays stable. The check under mount_lock stays for the case where
no propagation change is requested and namespace_sem isn't taken.
Callers get the same EINVAL as before.
Fixes:
|
||
|
Christian Brauner
|
8bacbf6be9 |
namespace: don't copy an unmounted mount tree
may_copy_tree() accepts a mount whose root is an nsfs or pidfs file before it checks whether that mount is mounted. This exception exists for nsfs and pidfs mounts. They are part of no mount namespace so neither check_mnt() nor is_mounted() would let them through and bind-mounting a nsfsfd or a pidfd wouldn't work. Problem is that exception also admits a bind mount of such a file when it was lazily unmounted and is only kept alive by a file descriptor: mount --bind /proc/self/ns/net /tmp/x exec 3< /tmp/x umount -l /tmp/x mount --rbind /proc/self/fd/3 /tmp/y With MS_REC or AT_RECURSIVE copy_tree() doesn't stop at the unmounted mount. It copies every mount that stayed attached to it as well. So if you have a bunch of overmounts over the nsfsfd or the pidfd the whole unmounted stack is copied. That's a bug. So refuse to walk an unmounted tree. Copying its root alone stays possible. So a bind mount of a namespace that is gone can still be recreated from a file descriptor on the old one. The file descriptor pins that mount and its root and __has_locked_children() only reads the direct children which don't leave the parent's list without namespace_sem. The internal mounts carry MNT_NS_INTERNAL instead of NULL and are still walked. Copying an unmounted tree recursively fails with EINVAL. Fixes: |
||
|
Christian Brauner
|
e2c6d32621 |
namespace: don't inherit MNT_UMOUNT in clone_mnt()
MNT_UMOUNT gets raised on a mount that umount_tree() or umount_one() has
taken out of its namespace. It's never cleared. __detach_mounts()
unhooks a mount that carries it instead of unmounting it.
disconnect_mount() lets an unmounted child stay attached only if its
parent carries it. propagate_umount() takes it as the sign that a mount
is going to be unmounted when it decides what else can be unmounted,
where a chain of locked mounts ends and where a for reparenting an
overmount.
clone_mnt() copies the flags of the source and only masks
MNT_INTERNAL_FLAGS which doesn't include MNT_UMOUNT. A mount that was
lazily unmounted but is kept alive by a file descriptor can still be
copied when its root is an nsfs or pidfs file because may_copy_tree()
accepts those before checking whether the source is mounted at all:
mount --bind /proc/self/ns/net /tmp/x
exec 3< /tmp/x
umount -l /tmp/x
mount --bind /proc/self/fd/3 /tmp/y
The mount on /tmp/y is attached to the caller's namespace with
MNT_UMOUNT already set and every copy of it inherits the flag again.
That causes a number of bugs:
- Unlink its mountpoint from a mount namespace where it isn't one.
__detach_mounts() takes it for a mount that was unmounted earlier,
unhooks it from its parent and drops the reference the namespace
holds on it. It stays in the namespace with one reference too few
and as its own parent, so it can't be unmounted through the file
descriptor anymore and the namespace never finds it again. When the
namespace goes away it's leaked and points to a freed struct
mnt_namespace that may_change_propagation() and anon_ns_root() read
on the next propagation change or mount_setattr() through the file
descriptor.
- Make it shared and unmount a peer. trace_transfers() follows the
flag from the peer into it and strips it from its peer group.
- Let a propagated umount reach it. gather_candidates() marks it with
T_UMOUNT_CANDIDATE but never puts it on the candidate list, so the
mark is never cleared and every later propagated umount skips it. If
it's the parent of a locked mount handle_locked() takes it as the
cutoff and commits the locked child that should have stayed and
disconnect_mount() leaves that child attached to it, unmounted under
a mounted parent, which is what disconnect_mount() exists to
prevent. When the namespace goes away umount_tree() visits the child
a second time. And once __detach_mounts() has made it its own parent
reparent() never finds the end of the chain of unmounted parents.
A clone is a new mount that has never been unmounted. Add MNT_UMOUNT
to MNT_INTERNAL_FLAGS so clone_mnt() never copies it. And warn when
umount_tree() reaches a mount that already has it set.
Fixes:
|
||
|
Christian Brauner
|
bb46094057 |
statmount: read the parent of an unmounted mount under mount_lock
statmount_mnt_basic() reads the ids of the parent through m->mnt_parent
with namespace_sem held for reading. That's fine for a mount that is
in a namespace. Everything above it is pinned by that namespace.
STATMOUNT_BY_FD also accepts a file descriptor on a mount that has
been unmounted on purpose and that only the file descriptor keeps alive.
If that mount stayed attached to its parent when it was unmounted the
parent is unmounted as well and nothing pins it for the child.
The final mntput() unmounts the children under mount_lock alone and
frees the parent afterwards. Concurrently, statmount_mnt_basic() reads
m->mnt_parent twice while that can happen and can easily dereference a
mount that's already gone.
Read the parent of an unmounted mount under mount_lock. The parent
can't be put while it is held and both ids come from the same mount. A
mounted mount's parent is pinned by the namespace and a mount can't
leave its namespace while namespace_sem is held, so the common case
takes no global lock.
Fixes:
|
||
|
Christian Brauner
|
476ef286a7 |
namespace: reset the old parent's ->overmount in mnt_change_mountpoint()
__umount_mnt() resets ->overmount of the parent when it unmounts the
mount that overmounts it. mnt_change_mountpoint() doesn't. It moves the
mount to its new parent and leaves the old one pointing at it.
For a mount that is moved beneath in attach_recursive_mnt() that doesn't
matter. The new mount has already reset ->overmount of the old parent by
the time the old top mount is moved onto it.
It matters for reparent() though. propagate_umount() moves a surviving
overmount onto the first ancestor that isn't going to be unmounted. The
mount it was taken from is unmounted immediately afterwards. If a file
descriptor keeps that unmounted mount alive ->overmount points at the
moved mount. That's a dangling pointer and UAF as the moved mount is
unmounted and freed.
do_lock_mount() follows ->overmount from the target path under
mount_lock. With a file descriptor on such a mount as the target of a
mount beneath topmost_overmount() walks into the freed mount and
do_lock_mount() takes references on whatever it finds there.
So reset ->overmount of the old parent in mnt_change_mountpoint() the
same way __umount_mnt() does. A mount's ->overmount is then always a
mount that is attached to it.
Fixes:
|
||
|
Christian Brauner
|
762a6411a5 |
Merge patch series "mount: a few gnarly fixes"
Christian Brauner <brauner@kernel.org> says: A short while ago I received a bug report about problematic behavior in the mount code and that let me to review and model a few corner cases including reference counting. The result is the following set of fixes: - Ensure that copies of unbindable mounts remain unbindable - Ensure that MOVE_MOUNT_SET_GROUP doesn't operate on unbindable mounts so we don't create an undefined state where a mount is both unbindable and a slave mount. - Ensure that we don't silently unmount busy mounts. This is nasty because it's an old bug from v4.13 where calculation of whether a synchronous (non-MNT_DETACH) umount would succeed drifted apart from the actual codepath that did the unmount. - Ensure that we don't silently unmount busy mounts. Same class, different mechanism. There's a race between the reference count check in propagate_mount_busy() (summing up per-cpu counters) and mntget(). Fixes and test for all. No test for the mntget() race because that needs aritifical delays. And fwiw, I think I another vomitorious series in the pipes. * patches from https://patch.msgid.link/20260923-work-mount-fixes-v1-0-f424cf8d3242@kernel.org: docs: update the unmount propagation rule fs: don't let a migrating task hide its reference from do_umount() selftests/filesystems: check that a busy propagated copy blocks a synchronous umount fs: don't silently unmount busy mounts selftests/move_mount_set_group: check that an unbindable target is refused mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount selftests/filesystems: check that a copied mount namespace keeps unbindable mount: keep a copied mount unbindable Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-0-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
3f5dc09141 |
docs: update the unmount propagation rule
Section 5f still describes the rule propagate_umount() used before
commit
|
||
|
Christian Brauner
|
7eb84d54fa |
fs: don't let a migrating task hide its reference from do_umount()
propagate_mount_busy() uses mnt_get_count() via do_refcount_check() to figure out whether a synchronous umount may proceed or not by summing the per-CPU mnt_count counter under mount_lock. The fastpath for mntget() and mntput() doesn't require that lock. A task that already holds a reference and takes another one on a cpu the loop has summed can get migrated and drops that reference again on another cpu. The loop might not have reached that cpu yet. That ends up undercounting. The original reference of the task is missed and umount() ends up succeeding with the file still open. The mount is destroyed later when the task mntput()s so it degrades to a lazy umount. That's basically the umount(2) variant of commit |
||
|
Christian Brauner
|
bba9960c07 |
selftests/filesystems: check that a busy propagated copy blocks a synchronous umount
A slave namespace moves an open_tree() copy of the shared tree beneath the propagated copy of the victim and keeps the descriptor. Check that: - umount(2) of the victim fails with EBUSY while the descriptor is open - the moved tree is still attached in the slave namespace afterwards - the umount succeeds once the descriptor is closed The test needs CAP_SYS_ADMIN and skips otherwise. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-6-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
6857e20901 |
fs: don't silently unmount busy mounts
The propagate_mount_busy() helper exists to decide whether a synchronous can suceed. It does that by looking at the copies of the victims in the mounts its parent propagates to. One of those functions from propagation hell. It skips a copy that has child mounts except for the case of a single child mount covering the copy's root. That algorithm used to be exact. It isn't anymore. Originally only childless copies were unmounted during propagation. And |
||
|
Christian Brauner
|
7541865c26 |
selftests/move_mount_set_group: check that an unbindable target is refused
MOVE_MOUNT_SET_GROUP must not accept an unbindable mount as the target. Cover both sources: - a slave, which used to leave the target unbindable and a slave at once - a shared mount, which used to silently drop the unbindable flag Check that the target is untouched after the refused call. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-4-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
a2b42babc9 |
mount: refuse MOVE_MOUNT_SET_GROUP on an unbindable mount
do_set_group() only accepts mounts as targets that are neither shared
nor a slave. That encompasses undindable mounts.
If the source mount is a slave mount the target mount will end up with
source's master as its master. It keeps T_UNBINDABLE. That means we get
a mount that is both unbindable and a slave:
mount --bind /tmp/src /tmp/src
mount --make-shared /tmp/src
mount --bind /tmp/src /tmp/b
mount --make-slave /tmp/b
mount --bind /tmp/src /tmp/c
mount --make-unbindable /tmp/c
move_mount(b, "", c, "", MOVE_MOUNT_SET_GROUP)
grep /tmp/c /proc/self/mountinfo
... /tmp/c ... master:646 unbindable ...
This property cannot be produced any other way. change_mnt_propagation()
drops the master when it makes a mount unbindable (iow, it becomes
private) and doesn't touch an unbindable mount when it is supposed to
turned into a slave mount.
End-result is that the unbindable-slave mount receives everything
propagated from its master's peer group while it can't be bind mounted
itself. Not sure what that's supposed to do.
On the other side: if the source mount is shared, the target mount drops
T_UNBINDABLE because because set_mnt_shared() clears T_SHARED_MASK.
So teach do_set_group() to refuse an unbindable target mount the same
way a shared or slave target mount is refused.
The main user of MOVE_MOUNT_SET_GROUP is CRIU which restores sharing
first and applies MS_UNBINDABLE afterwards. It never hits this.
Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-3-f424cf8d3242@kernel.org
Fixes:
|
||
|
Christian Brauner
|
0bab6b6d77 |
selftests/filesystems: check that a copied mount namespace keeps unbindable
Make a tmpfs unbindable, copy the mount namespace and check from the copy that: - a bind of the mount fails with EINVAL - a recursive bind fails as well - open_tree(OPEN_TREE_CLONE) of it fails - mountinfo still shows it as unbindable - a copy of the copy refuses the bind too The first case runs in the original namespace so the fixture stays honest about what it set up. Link: https://patch.msgid.link/20260923-work-mount-fixes-v1-2-f424cf8d3242@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
|
Christian Brauner
|
1ab8904ae6 |
mount: keep a copied mount unbindable
It's groundhog day. MNT_UNBINDABLE used to be set in in mnt->mnt.mnt_flags and was not part of MNT_INTERNAL_FLAGS. clone_mnt() copied it with all the other flags to the new mount. This guaranteed that the copy of an unbindable mount became unbindable as well. Unbindable copies that ended up as shared lost the unbindable property. But then commit |
||
Hui Peng
|
aa5e44b29f |
autofs: fix sbi->pipe file reference leak in autofs_kill_sb()
When autofs_fill_super() fails before clearing AUTOFS_SBI_CATATONIC (for
example, when find_get_pid() fails on an invalid pgrp mount option, or
when an fs_context is closed before mounting), deactivate_locked_super()
invokes autofs_kill_sb() -> autofs_catatonic_mode(sbi).
Because AUTOFS_SBI_CATATONIC is still set in sbi->flags,
autofs_catatonic_mode() returns early without calling fput(sbi->pipe),
permanently leaking the pipe struct file reference.
Explicitly release sbi->pipe in autofs_kill_sb() if it is still non-NULL
after autofs_catatonic_mode().
Fixes:
|
||
Drif Abdelmalek Mohamed Said
|
76d8e69724 |
dcache: unpoison the inline name buffer in __d_alloc()
syzbot reported:
BUG: KMSAN: uninit-value in dentry_string_cmp fs/dcache.c:291 [inline]
BUG: KMSAN: uninit-value in dentry_cmp fs/dcache.c:322 [inline]
BUG: KMSAN: uninit-value in __d_lookup_rcu+0x37d/0x5e0 fs/dcache.c:2522
dentry_string_cmp fs/dcache.c:291 [inline]
dentry_cmp fs/dcache.c:322 [inline]
__d_lookup_rcu+0x37d/0x5e0 fs/dcache.c:2522
lookup_fast+0x194/0xa40 fs/namei.c:1854
lookup_fast_for_open fs/namei.c:4545 [inline]
open_last_lookups fs/namei.c:4579 [inline]
path_openat+0x9ef/0x6540 fs/namei.c:4856
do_file_open+0x2aa/0x680 fs/namei.c:4888
do_sys_openat2+0x17c/0x390 fs/open.c:1395
do_sys_open fs/open.c:1401 [inline]
__do_sys_openat fs/open.c:1417 [inline]
__se_sys_openat fs/open.c:1412 [inline]
__x64_sys_openat+0x240/0x300 fs/open.c:1412
x64_sys_call+0x2445/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:258
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Uninit was stored to memory at:
copy_name fs/dcache.c:3031 [inline]
__d_move+0xd29/0x21f0 fs/dcache.c:3099
d_move+0x71/0xf0 fs/dcache.c:3147
vfs_rename+0x2619/0x2770 fs/namei.c:6085
filename_renameat2+0xa59/0x1230 fs/namei.c:6188
__do_sys_rename fs/namei.c:6232 [inline]
__se_sys_rename+0xc5/0x5c0 fs/namei.c:6228
__x64_sys_rename+0x78/0xb0 fs/namei.c:6228
x64_sys_call+0x329/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:83
do_syscall_x64 arch/x86/entry/syscall_64.c:63
do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Uninit was created at:
slab_post_alloc_hook mm/slub.c:4617 [inline]
slab_alloc_node mm/slub.c:4939 [inline]
kmem_cache_alloc_lru_noprof+0x376/0x1230 mm/s
__d_alloc+0x52/0x9f0 fs/dcache.c:1902
d_alloc+0x57/0x300 fs/dcache.c:1981
lookup_one_qstr_excl+0x19d/0x7a0 fs/namei.c:1806
__start_renaming+0x341/0x850 fs/namei.c:3888
filename_renameat2+0x625/0x1230 fs/namei.c:6163
__do_sys_rename fs/namei.c:6232 [inline]
__se_sys_rename+0xc5/0x5c0 fs/namei.c:6228
__x64_sys_rename+0x78/0xb0 fs/namei.c:6228
x64_sys_call+0x329/0x3ea0 arch/x86/include/generated/asm/syscalls_64.h:83
do_syscall_x64 arch/x86/entry/syscall_64.c:63
do_syscall_64+0x15d/0x3c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The race is between a concurrent open() and rename() of the same path.
__d_alloc() only stores the name itself and its terminating NUL, so the
rest of the inline buffer (d_shortname, DNAME_INLINE_LEN bytes) is left
uninitialized. copy_name(), called from rename(), copies that buffer as a
whole, so the uninitialized tail is propagated into the dentry that is
being moved. Meanwhile __d_lookup_rcu(), called from open(), is an
optimistic lockless lookup: it checks d_name.hash_len first and leaves the
seqcount retry to its caller, so it can end up comparing against a dentry
whose name a rename is rewriting in place, using a stale (longer) length.
The comparison then runs past the terminating NUL and reads bytes of the
uninitialized tail, which KMSAN reports.
The read is harmless by design: it stays inside the buffer, the name is
still NUL-terminated, and the result is thrown away by the seqcount retry.
It is not specific to KMSAN either - with CONFIG_DCACHE_WORD_ACCESS
enabled the very same bytes are read by read_word_at_a_time(), which is
__no_sanitize_or_inline and therefore invisible to KMSAN. KMSAN builds
only see the instrumented byte-at-a-time dentry_string_cmp() because
CONFIG_DCACHE_WORD_ACCESS is disabled when KMSAN is enabled on x86:
commit
|
||
Amir Goldstein
|
ae146bc1ab |
ovl: fix UAF in ovl_do_mkdir() debug print
ovl_do_mkdir() prints the input dentry with %pd after vfs_mkdir(). Since commit |
||
|
Christian Brauner
|
2d2a2d7aa9 |
super: make iterate_supers_type() deletion-safe
iterate_supers_type() drops sb_lock while invoking the callback and keeps
only a passive reference to the current superblock. That reference keeps
the object allocated, but does not keep its s_instances node linked.
After the iterator releases s_umount, final teardown can unlink the current
s_instances node. The iterator then advances through a reinitialized node.
With the current hlist it stops without visiting the remaining superblocks.
The unlink moved from generic_shutdown_super() to kill_super_notify(), but
the cursor lifetime has been unsafe since the helper was introduced.
The CIFS DFS lookup can consequently miss a matching superblock and return
-EINVAL.
Move removal from fs_supers to put_super(), alongside removal from
super_blocks, so a passive reference keeps both list nodes linked. Keep
the filesystem module reference until then, since unlinking s_instances
may touch type->fs_supers.
Make sget_fc() skip SB_DEAD superblocks before invoking test(), and set
SB_DEAD under sb_lock to serialize with those callbacks. This allows
kernfs to free its private information after kill_anon_super() returns.
Keep matching SB_DYING superblocks until SB_DEAD is set so concurrent
mounts still wait for teardown before retrying.
Fixes:
|
||
|
Christian Brauner
|
fabd324212 |
Merge patch series "Revert "put_mnt_ns(): leave mounts connected""
Christian Brauner <brauner@kernel.org> says:
Keeping every mount of a dying namespace attached to its parent changes
who owns it. An attached but unmounted mount is owned by its parent: the
parent's final mntput() is what drops the child's reference and takes
its superblock down. Before that commit only the namespace root was in
that position and every other mount dropped its own reference in
namespace_unlock().
That ownership rule turns any reference from a child's superblock back
to one of its ancestors into a cycle. The obvious case is a loop device:
unshare -m sh -c 'mount -o loop /var/tmp/img /var/tmp/mp'
mount(8) opens the image from inside the new namespace, so the loop
device's backing file pins that namespace's copy of the root mount.
When the namespace dies the loop mount stays attached to that copy and
is owned by it. The copy can't reach zero because the backing file
holds it. The backing file is only put once the ext4 superblock
releases the block device and autoclear runs, and that needs the loop
mount to go first. Nothing breaks the cycle. The superblock, the loop
device and, since the root copy keeps all of its children.
systemd-sysext sets up its loop devices in a private mount namespace and
relies on autoclear when that namespace goes away, which is how this was
found. Anything that holds a file on an ancestor from a mounted
filesystem has the same problem. That includes ecryptfs lower paths,
erofs file-backed mounts, fuse passthrough backing files.
That's a bigger fix and not an -rc change. Revert.
* patches from https://patch.msgid.link/20260917-work-put_mnt_ns-revert-v1-0-34d9b8679e68@kernel.org:
Revert "put_mnt_ns(): leave mounts connected"
Revert "selftests/filesystems: add mntns cleanup test"
Link: https://patch.msgid.link/20260917-work-put_mnt_ns-revert-v1-0-34d9b8679e68@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
||
|
Christian Brauner
|
2e2142a35d |
Revert "put_mnt_ns(): leave mounts connected"
This reverts commit |
||
|
Christian Brauner
|
76ebb69da6 |
Revert "selftests/filesystems: add mntns cleanup test"
This reverts commit
|
||
|
Christian Brauner
|
f615c80a5d |
Merge patch series "binfmt: fixes for kres reports"
Christian Brauner <brauner@kernel.org> says: I asked Chris to run his kres tooling on the binfmt with bpf changes merged for this cycle. It found two issues that are fixed in this series. I reproduced both of them. * patches from https://patch.msgid.link/20260918-work-binfmt_misc-fixes-v1-0-647b24bc1c46@kernel.org: binfmt_misc: fix racy checks in bpf set_interp kfuncs binfmt_misc: fix OOB read in bpf_binprm_select_interp() Link: https://patch.msgid.link/20260918-work-binfmt_misc-fixes-v1-0-647b24bc1c46@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> |
||
Chris Mason
|
1970fc4ecb |
binfmt_misc: fix racy checks in bpf set_interp kfuncs
bpf_binprm_set_interp() tests path[0] != '/' on the buffer its load
program passes and then reads the same buffer again to copy it with
kmemdup_nul(). The buffer can be a BPF map value that another CPU
rewrites between the two reads. If byte 0 is overwritten in that
window, the kfunc stages a relative or empty interpreter path. The
staged path is not checked again, so open_exec() resolves a relative
path against the working directory of the task doing the exec.
bpf_binprm_set_interp_arg() has the same pattern for its "!len" test
and can stage an empty argument, which the interpreter then receives
as an empty argv entry.
The verifier checks the path and path__sz pair with BPF_READ |
BPF_WRITE, so a writable array map value is an accepted argument.
bpf(BPF_MAP_UPDATE_ELEM) on an array map copies the new value over the
old one in place and takes no lock. Both kfuncs are KF_SLEEPABLE and
allocate with GFP_KERNEL between the test and the copy, so the task
can sleep inside the window:
load program bpf(BPF_MAP_UPDATE_ELEM)
bpf_binprm_set_interp()
strnlen(path, path__sz)
path[0] != '/' is false
kmemdup_nul(path, len, GFP_KERNEL)
allocation may sleep
array_map_update_elem()
copy_map_value()
rewrites byte 0
copy reads path again
bm_bpf_stage_selection()
The test in the load program's column proves what byte 0 held only at
the moment the test ran. The map update takes no lock, so it can store
to byte 0 right after. kmemdup_nul() then copies the rewritten bytes,
and bm_bpf_stage_selection() publishes them as bprm->bpf_interp.
The staged path is not checked again on its way to open_exec():
load_misc_binary()
entry_select_interpreter() returns bprm->bpf_interp unchanged
build_interp_argv()
copy_string_kernel() copies it as argv[0]
bprm_change_interp()
kstrdup()
entry_open_interpreter()
open_exec() unless a bound file is staged or
the entry is an 'F' entry
None of these functions tests the first byte, and load_misc_binary()
hands the pointer to nothing else.
In bpf_binprm_set_interp_arg(), strnlen() finds a non-zero len, a NUL
is then stored to byte 0, and build_interp_argv() later copies the
empty bprm->bpf_interp_arg with copy_string_kernel().
The handler's own load program has to pass a writable map value, and
something has to store into it while the kfunc runs. The allocation can
sleep inside the window, and with a BPF_F_MMAPABLE array the store is a
plain user space write into the mapped value, so a loop can hit it
without a single bpf() call.
Check the private copy in both kfuncs, so that the string that gets
staged is the string that was checked. bpf_binprm_select_interp()
already looks its name up in a private copy for the same reason. The
remaining tests work on path__sz, arg__sz or the local len, and the
copy length is len, so the copy stays inside the extent the verifier
checked.
Results of bpf_binprm_set_interp() with the check on the copy:
- A NUL stored to byte 0 fails interp[0] != '/' and gets -EINVAL.
- For len == 0, kmemdup_nul() returns an empty string, so an empty path
still gets -EINVAL.
- A NUL stored further into the string only shortens it to another
absolute path, or another non-empty argument, that the program could
have passed anyway.
- A path that both lacks the leading '/' and is PATH_MAX or longer now
gets -ENAMETOOLONG instead of -EINVAL.
- A path that is empty or lacks the leading '/' is now rejected after
the copy rather than before it, so such a call makes an allocation
and returns -ENOMEM instead of -EINVAL if that allocation fails.
bpf_binprm_set_interp_arg() still rejects an empty argument before
allocating, so its results are unchanged apart from the raced case
fixed here.
Both new checks run before the previously staged string is freed or
replaced. A failing call frees only its own allocation and leaves the
earlier selection in place, as the -ENOMEM path already does.
Fixes:
|
||
|
Chris Mason
|
4f948b5949 |
binfmt_misc: fix OOB read in bpf_binprm_select_interp()
bpf_binprm_select_interp() checks the name its load program passes with
strnlen(name, name__sz) and then hands the same buffer to
binfmt_misc_find_interp(), which compares it with an unbounded strcmp().
The buffer can be a BPF map value that another CPU rewrites between the
two reads. If the terminating NUL is overwritten in that window, strcmp()
reads past the name__sz bytes the verifier checked. That is an
out-of-bounds read of up to 31 bytes of whatever follows the checked
name__sz bytes.
The verifier checks the name and name__sz pair with BPF_READ | BPF_WRITE,
so a writable array map value is an accepted argument.
bpf(BPF_MAP_UPDATE_ELEM) on an array map copies the new value over the
old one in place and takes no lock. The NUL that strnlen() finds can be
overwritten before strcmp() reads the buffer again:
CPU0 CPU1
bpf_binprm_select_interp()
strnlen(name, name__sz)
finds the NUL inside name__sz
bpf(BPF_MAP_UPDATE_ELEM)
array_map_update_elem()
copy_map_value()
overwrites the NUL
binfmt_misc_find_interp()
strcmp(interp->name, name)
reads past name__sz
strnlen() proves that a NUL lies inside name__sz only at the moment it
runs. The map update on CPU1 takes no lock, so it can store over the NUL
right after. The lookup on CPU0 then walks the live buffer again, once
per bound interpreter:
fs/binfmt_misc.c:binfmt_misc_find_interp
list_for_each_entry(interp, interps, list)
if (!strcmp(interp->name, name))
return interp;
strcmp() stops at the first mismatch or at the end of interp->name.
bm_entry_add_interp() caps a bound name at BINFMT_MISC_INTERP_NAME_MAX
(32) bytes, so strcmp() reads at most 33 bytes of name. The smallest
name__sz the kfunc accepts is 2, which leaves up to 31 bytes read beyond
the checked extent. The handler's own load program has to pass a
writable map value, and something has to store into it while the kfunc
runs. The window between strnlen() and strcmp() is short, but with a
BPF_F_MMAPABLE array the store is a plain user space write into the
mapped value, so a loop can hit it without a single bpf() call.
Copy the name into a stack buffer of BINFMT_MISC_INTERP_NAME_MAX + 1
bytes, terminate it, and look up the copy. The memcpy() length is below
name__sz, so the copy stays inside the extent the verifier checked, and
the BPF buffer is not read again afterwards.
Return -ENOENT first for a name longer than BINFMT_MISC_INTERP_NAME_MAX.
bm_entry_add_interp() rejects a longer name, and the only other binding
site attaches the empty name. No entry can bind such a name, so that
lookup already ended in -ENOENT and no result changes.
Check the first byte of the copy and return -EINVAL if it is NUL, as the
existing "!len" test does for an empty name. Only an 'F' entry binds the
empty name and a 'B' entry cannot carry 'F', so without that check a
racing store of NUL to byte 0 would look up a name no entry binds and
end in -ENOENT rather than -EINVAL. A NUL stored further into the name
only shortens it to another name the program could have passed anyway.
binfmt_misc_find_interp() itself is left alone: entry_attach_interpreter()
calls it with a kernel string, and this kfunc now calls it with a private
copy.
Fixes:
|
||
|
Christian Brauner
|
5179241521 |
fs: don't create the private nullfs mount under namespace_sem
init_mount_tree() mounts the mutable rootfs on top of nullfs via
LOCK_MOUNT_EXACT(). That declares a pinned mountpoint with a cleanup
attribute in the scope of the whole function so the nullfs root inode
lock and namespace_sem are only dropped when init_mount_tree() returns.
This became a problem when the private nullfs instance for kthreads was
added. kern_mount() allocates a new superblock and alloc_super() takes
the new s_umount with SINGLE_DEPTH_NESTING and then shrinker_mutex via
shrinker_alloc(). Doing that with namespace_sem held teaches lockdep the
dependency
namespace_sem -> s_umount/1 -> shrinker_mutex
With CONFIG_SHRINKER_DEBUG shrinker_debugfs_rename() takes the debugfs
directory inode lock under shrinker_mutex every time a block device is
mounted and lock_mount_exact() takes namespace_sem under the inode lock
of the mountpoint for every mount. So mounting anything on debugfs,
e.g. the tracefs automount on /sys/kernel/debug/tracing, closes the
cycle:
WARNING: possible circular locking dependency detected
7.3.0-rc3+ #17 Not tainted
------------------------------------------------------
rasdaemon/4449 is trying to acquire lock:
(namespace_sem){++++}-{4:4}, at: lock_mount_exact+0x4c/0x308
but task is already holding lock:
(&sb->s_type->i_mutex_key#17){++++}-{4:4}, at: lock_mount_exact+0x3c/0x308
which lock already depends on the new lock.
...
Chain exists of:
namespace_sem --> shrinker_mutex --> &sb->s_type->i_mutex_key#17
This can't actually deadlock. init_mount_tree() runs single-threaded
during early boot before any other task exists and nothing allocates a
superblock under namespace_sem after that. But lockdep can't know that
and disables itself for the rest of the boot.
Move mounting the rootfs on top of nullfs into a helper so the locks
are dropped when it returns.
Fixes:
|
||
Patrick Lu (Anthropic)
|
f6988c9067 |
writeback: bound cleanup_offline_cgwb() rescans by rotating scanned inodes
cleanup_offline_cgwb() prepares at most WB_MAX_INODES_PER_ISW inodes
per call and is called again until the dying wb is drained, but every
call walks wb->b_attached and then wb->b_dirty_time from the same end.
Inodes already prepared (they stay on the list with I_WB_SWITCH set
until the switch worker runs) and inodes that cannot be switched
(I_FREEING, I_WILL_FREE, !SB_ACTIVE, DAX, already on the target wb)
stay where they are, so each pass rescans a growing run of them under
wb->list_lock and a full drain is quadratic in the number of inodes on
the list. With ~17M inodes attached to one dying cgwb we saw this end
in soft lockups, with CPUs reported stuck for 21-48s.
Walk both lists from the oldest end and move every scanned inode to
the newest end, so the next pass starts where the previous one stopped
and the drain becomes linear. b_attached is unordered, so nobody sees
the reorder there. b_dirty_time is ordered by dirtied_when, but the
oldest unscanned inode stays at the end move_expired_inodes() picks
from, sync takes the whole list regardless of order, and prepared
inodes leave the list as soon as the switch work runs and get a new
dirtied_time_when on the new wb anyway, so the only inodes left out of
order are the ones that can never switch (DAX), and only on the dying
wb.
Fixes:
|
||
Julian Sun
|
7459c02187 |
fs: avoid repeated scans in evict_inodes()
We observed hung tasks when users attempted to unmount a filesystem
after its disk had been removed while still in use. During device
removal, fs_bdev_mark_dead() calls evict_inodes() while holding s_umount.
Each time evict_inodes() drops s_inode_list_lock to reschedule, it
restarts the walk from the head of s_inodes. With many referenced inodes
at the head of the list, these restarts repeatedly scan the same inodes
without reclaiming them. This can keep s_umount held for a long time,
blocking concurrent umount attempts and triggering hung-task reports.
Keep the current inode, already marked I_FREEING, out of the disposal
batch until s_inode_list_lock is reacquired. Resume the walk from this
inode and dispose of it in a later batch or at the end of the walk.
The zero-refcount and state checks under i_lock allow this walker to
claim the inode by setting I_FREEING and removing it from the LRU.
Other reclaimers skip the inode, leaving this walker responsible for
eviction. Only evict() removes it from s_inodes, so keeping it out of
the disposal batch ensures that it remains on the list while the lock
is dropped. After reacquiring the lock, reading its current next pointer
accounts for concurrent removal of following inodes.
The existing inode lifetime rules prohibit acquiring a reference to an
inode marked I_FREEING or I_WILL_FREE. __iget() requires its caller to
hold i_lock and establish that taking a reference is valid. Inode lookup
and igrab() check these flags under i_lock when acquiring a reference
from zero. ihold() requires an existing reference, which would keep
i_count nonzero and prevent this walker from claiming the inode. These
rules already allow iput_final() and the inode shrinker to release
i_lock after setting I_FREEING and before eviction completes.
A temporary __iget() reference would also keep the inode on the list,
but its release must preserve last-reference handling. Another user can
acquire a reference, update lazy timestamps and drop its reference while
the pin is held. If the pin becomes the last reference, dropping it with
atomic_dec_and_test() and evicting directly bypasses iput()'s lazytime
handling and can lose those timestamp updates.
Releasing the pin with iput() preserves that handling, but does not
guarantee eviction. fs_bdev_mark_dead() runs with SB_ACTIVE set, so iput()
may retain the inode in cache, whereas evict_inodes() must evict eligible
zero-reference inodes. The inode may also have been freed when iput()
returns, so the walker cannot then use it to force eviction. Using
I_FREEING preserves the existing eviction behavior without introducing
an additional last-reference transition.
The xfstests auto group passed on ext4 and XFS with known unrelated
failures excluded. No new issues were observed, and the previously
reproducible hung task no longer occurs with this patch.
Fixes:
|