| Age | Commit message (Collapse) | Author |
|---|
|
Pull fsverity fix from Eric Biggers:
"Fix a regression from commit f77f281b6118 ("fsverity: use a hashtable
to find the fsverity_info")"
* tag 'fsverity-for-linus' of git://git.kernel.org/pub/scm/fs/fsverity/linux:
fsverity: RCU-delay the freeing of struct fsverity_info
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
Pull vfs fixes from Christian Brauner:
"This contains fixes for the current development cycle.
All of them came out of a review of the mount code that started with a
bug report. The review modeled the corner cases of mount propagation,
unmounting and mount reference counting and turned up a lot of bugs.
Most of them years old. Most fixes come with a selftest.
- Rework connected mounts.
A mount that is unmounted together with its parent can stay
attached to the parent to keep its mountpoint covered. That happens
when the mountpoint is removed with rmdir(), unlink() or rename(),
when a detached tree is dissolved, and for locked mounts in any
umount that isn't synchronous, including the teardown of their
mount namespace. The parent then owns the child and drops it on its
own final mntput(). So any reference from the child's superblock
back to one of its ancestors becomes a cycle that is never freed.
A loop device backed by an image on a tmpfs and mounted on that
same tmpfs is enough. Remove the directory the tmpfs is mounted on
from the host, let the container's mount namespace exit, and the
loop device, the tmpfs and the filesystem on the loop device are
leaked for good. The same works with autofs, zram, ecryptfs,
binfmt_misc, fuse passthrough, zloop, a mass storage gadget and md,
and the selftests have reproducers for them. This has been possible
since v4.1. It is also why "put_mnt_ns(): leave mounts connected"
was reverted in -rc5. Keeping every mount of a dying mount
namespace connected made these cycles trivial to create.
Every unmounted mount is now detached from its parent. Where the
mountpoint has to stay covered the mount leaves a cover on the
parent instead, allocated together with the mount. A lookup on the
unmounted parent that hits a cover finds an empty immutable
directory or file on the private nullfs instance. Nothing leads
from a cover to another mount, so no unmounted mount owns another
one and no cycle can form.
This is visible to userspace. A formerly connected mount can no
longer be reached through its unmounted parent and ".." inside it
leads nowhere, as for every other lazily unmounted mount.
With that the private nullfs instance becomes reachable from
userspace, so it now refuses mounts on top, is mounted read-only,
and refuses fsnotify marks and file locks. Its inodes are shared by
every holder and a watch or a lock would otherwise reach across
users. may_decode_fh() now also decides its subtree check under a
single mount_lock hold, as a racing umount could otherwise let it
decode into what a locked child covered.
- umount:
* Don't silently unmount busy mounts.
Since v4.13 propagate_umount() takes down propagated copies of
the victim with children as long as each child is an overmount or
another copy of the victim, but propagate_mount_busy() only ever
checked copies without children or with just an overmount. A
container that moved a tree beneath its copy of a host mount lost
that tree from under its open file descriptor to a plain umount()
on the host. propagate_mount_busy() now applies the same rules,
walking each chain of copies once.
* Don't let a migrating task hide its reference from umount().
mnt_get_count() sums the per-cpu counters under mount_lock but
the mntget() and mntput() fast paths don't take it. A task that
takes a reference on a cpu the sum has already passed and drops
it after migrating to one the sum hasn't reached yet hides the
reference it held to begin with, and umount() succeeds with the
file still open. Gets and puts now live in separate per-cpu
counters and all puts are summed before all gets with a full
barrier in between, the way srcu_readers_active_idx_check() does
it. mntget() is unchanged and mntput() gains an smp_wmb().
* Check each submount for references right before unmounting it.
shrink_submounts() and mark_mounts_for_expiry() checked all their
victims up front. Unmounting the first could move a busy
overmount to where the next victim's propagated copy is looked up
and it was then unmounted without a check.
* Never expire a locked mount.
A shrinkable mount moved beneath a locked mount with
MOVE_MOUNT_BENEATH takes over the lock, and umount() of an
unlocked ancestor expired it and revealed what it covered. That
umount() now fails with EBUSY as it does for any other locked
child. A lazy umount still takes the whole tree.
- Overmounts and locked mounts:
* Unhash a dentry before detaching the mounts on it.
unlink(), rmdir() and rename() detach the mounts on the victim
but only d_delete() it once its inode is unlocked, a window that
includes an expedited RCU grace period. In between, a lookup from
a mount namespace in which the dentry is a mountpoint found it
hashed, positive and uncovered. Drop the dentry first, as
d_invalidate() already does.
* Don't reveal overmounted entries in refwalk.
A refwalk that had grabbed the dentry before the unlink never
rechecked it the way rcuwalk does with d_seq and mount_lock.
Without any artificial widening three walkers read the covered
file 27 times in a minute. step_into() now fails an unhashed
dentry marked DCACHE_CANT_MOUNT with -ESTALE and the walk is
retried.
* Keep covered mounts covered in OPEN_TREE_NAMESPACE.
Creating such a mount namespace only takes a user namespace and
the copy followed bind mount rules: no children without
AT_RECURSIVE and no unbindable mounts with it. An unprivileged
user could see what mounts covered in the source, such as the
parts of /proc and /sys that container runtimes mask. If the
caller doesn't own the source mount namespace a non-recursive
copy of a mount with something mounted below the requested
directory is now refused and a recursive copy includes unbindable
mounts, the way unshare() copies.
* Keep the lock on a mount that a propagated copy is moved beneath.
MNT_LOCKED moved to any mount that ended up beneath a locked
mount, propagated copies included. A host mount and umount on a
directory covered by a locked mount in a less privileged mount
namespace left that cover unlocked for the namespace's owner to
remove. Only mounts the caller places beneath take over the lock
now.
* Handle mount locking for automounts correctly. Which copies to
lock was decided by the mount namespace of the task that
triggered the automount. A task in a user namespace that
triggered one on a host mount through a file descriptor got the
host's own automount locked while its own copy stayed unlocked
and could have nosuid, nodev and noexec cleared. Use the owner of
the mount namespace the mount lands in.
- Use-after-free and crashes:
* Refuse an automount below a mount that is in no namespace.
The private clones overlayfs uses for its layers have the
MNT_NS_INTERNAL error pointer as their namespace, which
finish_automount() let through and count_mounts() dereferenced. A
fanotify filesystem mark on an overlayfs lower layer hands out
file descriptors on such a clone. With debugfs as the lower layer
opening "tracing" oopses with namespace_sem held for writing and
every mount operation on the system blocks from then on.
* Reset the old parent's ->overmount in mnt_change_mountpoint().
When propagate_umount() moved an overmount off a mount that a
file descriptor kept alive, MOVE_MOUNT_BENEATH through that
descriptor later followed the stale pointer into the freed
overmount.
* statmount() with STATMOUNT_BY_FD and pivot_root() read the parent
of a mount that may be unmounted and only held by a file
descriptor, while the parent's final mntput() can free it.
statmount() now reads it under mount_lock and pivot_root() first
checks that both mounts are in the caller's mount namespace.
* Queue a mount only once for mount notifications. A mount
reparented by one umount_tree() and taken down by the next under
the same namespace_sem hold, as in shrink_submounts(), was queued
twice. That cut the mounts queued in between out of notify_list
while it still pointed at them, and once they were freed every
later mount operation walked freed memory.
* Don't let a pseudo dentry become the root of a mount. A bind
mount of a bpf token file did that with a DCACHE_NORCU dentry,
which is freed without an RCU grace period while lockless path
walks may still look at it. Refuse to clone such a mount.
* Don't inherit MNT_UMOUNT in clone_mnt(). A bind mount of a lazily
unmounted nsfs or pidfs mount through its file descriptor started
out flagged as unmounted. Among other things __detach_mounts()
then dropped the namespace's reference on it, the mount outlived
its namespace and mount_setattr() through the descriptor read the
freed namespace. A recursive bind mount of such a mount also
copied the unmounted stack still attached to it. That now fails
with EINVAL, copying the mount itself still works.
* Remove the fsnotify marks of a mount namespace in free_mnt_ns()
instead of the RCU callback that frees the namespace, where
taking the group mutexes meant sleeping in softirq context.
- Propagation and copies:
* Keep a copied mount unbindable. Since v6.17 clone_mnt() didn't
copy the unbindable flag, so every mount namespace created with
CLONE_NEWNS had bindable copies of all unbindable mounts. This
had been fixed once before.
* Refuse MOVE_MOUNT_SET_GROUP on an unbindable mount. It made the
mount an unbindable slave, a state nothing else can produce, or
silently dropped the unbindable flag. CRIU applies MS_UNBINDABLE
after restoring sharing and isn't affected.
* Check a recursive bind mount for mount namespace loops.
Recursively bind mounting a tree from another mount namespace
could put a mount of a namespace's file inside that same
namespace, which then pins itself and all its mounts. Repeating
it leaks without limit, the reproducer took Shmem from 380 kB to
65916 kB. The copy is now checked with check_for_nsfs_mounts()
before it is grafted, as move_mount() does.
* Look at the topmost mount for a mount namespace file.
attach_recursive_mnt() never looked at the topmost mount of the
source's chain of overmounts. If that was the chain's only mount
namespace file an existing mount at a propagated destination got
buried below the root of the nsfs file where no path walk reaches
it.
* Don't put a mountpoint on a dentry that's being removed.
attach_recursive_mnt() makes a mountpoint of the source's root
without its inode lock, so a racing rmdir() of that directory
could leave a mount on it that nothing ever detaches.
d_set_mounted() now checks cant_mount() as well.
- nullfs:
* Take no inode lock for readdir of an immutable directory.
The root of every empty mount namespace is the same nullfs
directory and iterate_dir() held its i_rwsem across
->iterate_shared(). A reader whose buffer faults on a FUSE mount
of its own holds it for as long as its server wants, and with an
exclusive locker queued behind it every lookup that misses the
dcache, every create and every mount in that directory waits. One
user of an empty mount namespace stalls all others. Directories
with the new FOP_IMMUTABLE flag skip the lock.
* Refuse to reconfigure internal superblocks through fspick(),
MS_REMOUNT or the read-only remount that a synchronous umount()
of the root does. For nullfs only root in the initial user
namespace could do it, but the superblock is shared by every
mount namespace and the flags showed up in statfs() for all of
them.
* Don't update the access time on nullfs and refuse F_SET_RW_HINT
on an immutable inode.
- unshare: Free an nsproxy that was never installed with
nsproxy_free() when set_cred_ucounts() fails. put_nsproxy() dropped
active references that were never taken, which triggered a warning
and hid the caller's own namespaces from listns().
- Smaller changes: mount_setattr() checks the target before it walks
the tree to allocate peer group ids, unshare() puts the old
fs_struct before the old namespaces, dissolve_on_fput() drops the
file's reference to the tree itself, disconnect_mount() is
simplified and the documentation of the propagated unmount rule is
brought up to date"
* tag 'vfs-7.3-rc7.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: (58 commits)
namespace: simplify disconnect_mount()
selftests/filesystems: test covered mounts
namespace: rework connected mounts
nullfs: add an empty immutable regular file
selftests/filesystems: check that reading the root of an empty mount namespace stalls nobody
selftests/filesystems: add a helper that holds a readdir in a page fault
readdir: take no inode lock on an immutable directory
nullfs: refuse file locks
fsnotify: let a filesystem refuse marks on its objects
namespace: nothing is mounted on or written through knullfs
namespace: keep the private nullfs instance in knullfs
fhandle: decide the subtree check under mount_lock
selftests/filesystems: check that an automount below an overlay layer is refused
selftests/filesystems: check the atime of the empty mount namespace root
selftests/filesystems: check that a lock lands on the right mount and stays
namespace: keep the lock on a mount that a propagated copy is moved beneath
namespace: never expire a locked mount
nullfs: don't update the access time
namespace: handle mount locking for automounts correctly
namespace: refuse an automount below a mount that is in no namespace
...
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
Pull ext4 fixes from Ted Ts'o:
"Mark the ext4 data=journal feature as being deprecated and will be
removed in 2028.
Also designate the primary branch that Sashiko and other tools use to
find the primary development branch in the ext4 tree"
* tag 'ext4_for_linus-7.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4:
ext4: mark data=journal as deprecated and will be removed in January 2028.
MAINTAINERS: name the ext4 dev branch
|
|
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
Pull btrfs fixes from David Sterba:
- fix command queuing and cleanup in encoded read/write ioctls
- fix root and transaction association to avoid unnecessary lock
contention and transaction start
- properly handle replacing multiple xattrs in the same item
- in scrub, fix root reference leak after reporting an unresolved file
path
* tag 'for-7.3-rc6-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux:
btrfs: fix lost error return value in btrfs_listxattr()
btrfs: fix xattr replace when multiple xattrs are packed in the same item
btrfs: don't stash io_uring encoded data across -EAGAIN
btrfs: unlock inode and extent in caller when io_uring read extent fails
btrfs: free iov when btrfs_uring_read_extent() fails
btrfs: always return -EIOCBQUEUED after btrfs_uring_read_extent_endio
btrfs: scrub: fix local_root reference leak in scrub_print_warning_inode()
btrfs: clear BTRFS_ROOT_IN_TRANS_SETUP on early exit from record_root_in_trans()
|
|
Rename disconnect_mount() and simplify it.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
UMOUNT_CONNECTED as implemented allows for the creation of reference
count cycles. Here's a simple example
mkdir /x; mkfifo /ready /go
unshare -m sh -c 'mount -t tmpfs tmpfs /x
truncate -s 8M /x/img; mkfs.ext4 -q /x/img
dev=$(losetup -f --show /x/img)
mkdir /x/mp; mount $dev /x/mp
echo $dev > /ready; read r < /go' &
read dev < /ready
rmdir /x
echo > /go
wait
losetup -d $dev
losetup -a
Take a directory /x on the host, create a new mount namespace, mount a
tmpfs on /x, use a file on that tmpfs as the backing file for a loop
device, mount that loop device on that tmpfs. Now rmdir /x on the host.
This will lazily unmount the mount on top of /x in the container with
UMOUNT_CONNECTED. Once the namespace exits nothing references the mount
anymore. Now the tmpfs is pinned by the backing file of the loop device
and the loop mount is owned by the tmpfs superblock.
Fun fact, such cycles can be formed by at least the following
subsystems and I have added reproducers for all of them:
(1) a loop mount P from an image on a tmpfs next to it, so that P's
death shows as the loop device giving up its backing file
(2) autofs with a FIFO on P as its pipe, zram with a device node on P as
its writeback device, both on a minix image since vfat has neither
(3) ecryptfs with its lower directory on P, under a passphrase token
added to the session keyring
(4) binfmt_misc in a new user namespace with an 'F' interpreter on P
(5) a fuse server that answers FUSE_INIT with passthrough on and
registers a file on P as a backing file
(6) zloop with its zone files in a directory on P
(7) a mass storage gadget on the dummy UDC with its LUN file on P,
mounted from the SCSI disk the gadget shows up as
(8) md with a RAID1 of one loop device and its bitmap file on P, which
skips while SET_BITMAP_FILE has no way to succeed
(9) rmdir of P's mountpoint from the parent, then the child exits, then
the device must be free and LOOP_CLR_FD must release the file
The underlying mechanism is UMOUNT_CONNECTED (MNT_LOCKED falls into the
same class). With UMOUNT_CONNECTED an unmounted mount stays attached to
its parent. This is used to protect revealing the underlying mount and
is a non-negotiable security mechanism. So now the parent owns that
mount and is put on the parent's final mntput(). That moves it to
mnt_stuck_children and ultimately it's cleaned up by cleanup_mnt().
The fact that ownership of the child mount gets transferred to the
parent turns every reference from a child's superblock back to one of
its ancestors into a cycle.
Don't keep the child attached at all. What the parent needs is that a
lookup at the child's mountpoint keeps finding some mount, not the child
itself and it's not a guarantee we have given really.
When an unmounted mount would have stayed attached to its unmounted
parent disconnect it like every other unmounted mount and leave a
marker behind.
A lookup on the parent that misses the mount hash and hits a marker
finds knullfs. Either a file or a directory. Nothing leads from a marker
to any other mount.
The marker is owned by the parent and dropped by the parent's final
mntput() or by __detach_mounts() when the mountpoint is deleted from
under it. It is allocated together with the mount.
With that every unmounted mount is a root and holds only its own
reference which namespace_unlock() drops. No unmounted mount owns
another one. A superblock that pins an ancestor can't form a cycle.
It has a visible change. Mounts left connected (rmdir etc.) used to stay
traversable through the parent for as long as something held the parent.
Now it is detached with the umount. It lives as long as something
references it but it isn't reachable through the parent anymore and ".."
inside it leads nowhere which is the same as for every other lazily
unmounted mount.
Link: https://gist.github.com/mvo5/63ef46482349f3b1c3957d463a0c9c6f
Link: https://patch.msgid.link/20261002-work-mount-cover-v1-2-232a8f52b43c@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Add nullfs_new_file() to allocate an empty immutable regular file on a
nullfs instance as a dentry of its own. It is never hashed under the
root and so can't be found by lookup. Reads return nothing, changes are
refused, file locks, leases and delegations are refused as.
Link: https://patch.msgid.link/20261002-work-mount-cover-v1-1-232a8f52b43c@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
If the input buffer does not have enough space to store the current xattr,
we set 'iter_ret' to -ERANGE and then do "break", but that only exits the
while loop over the xattrs in the current btrfs_dir_item, and then we
continue the btrfs_for_each_slot() iteration, which overwrites the value
of 'iter_ret' causing us to lose the error return value and proceed as if
the buffer has enough space.
Fix this by returning -ERANGE directly (the path is automatically freed)
instead of breaking from the while loop.
Fixes: 184b3d190087 ("btrfs: use btrfs_for_each_slot in btrfs_listxattr")
Assisted-by: LLM (found the bug)
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If we have a btrfs_dir_item item that packs multiple xattrs and then we
replace the value of one of them (with the setxattr(2) family of syscalls)
with another value of a different size, we end up not having a fully
initialized btrfs_dir_item, resulting in a corruption that the tree
checker will detect at extent buffer writeback time.
This is because in btrfs_setxattr() when we find a btrfs_dir_item with
multiple xattrs (due to the crc32c hash of their name being the same)
we delete one of the xattr items (btrfs_dir_item) and then insert a new
one, but the deletion and insertion results in shifting existing data in
the leaf and therefore when the new value of a xattr has a different size,
the new btrfs_dir_item is placed in a leaf section that was not
initialized and we only copy the value's data and set the value's length
in the new btrfs_dir_item, without setting the name, the name's length,
the key (which must be all zeroes for xattrs), flags (BTRFS_FT_XATTR) and
transaction ID.
The following script reproduces the issue:
$ cat test.sh
#!/bin/bash
DEV=/dev/sdi
MNT=/mnt/sdi
mkfs.btrfs -f $DEV
mount $DEV $MNT
touch $MNT/testfile
# Add two xattrs that, on btrfs, have the same hash (crc32c) for their
# name and therefore are packed into the same btrfs_dir_item.
setfattr -n user.foobar -v 123 $MNT/testfile
setfattr -n user.WvG1c1Td -v qwerty $MNT/testfile
# Verify the xattrs are present.
echo "xattrs before:"
getfattr --absolute-names --dump $MNT/testfile
# Now replace the value of the foobar xattr with a significantly larger
# value.
setfattr -n user.foobar -v abcdefghijklmnopqrstuvwxyz $MNT/testfile
# Check the xattrs have the expected values.
echo "xattrs after:"
getfattr --absolute-names --dump $MNT/testfile
umount $MNT
Running it:
$ ./test.sh
(...)
xattrs before:
# file: /mnt/sdi/testfile
user.WvG1c1Td="qwerty"
user.foobar="123"
xattrs after:
# file: /mnt/sdi/testfile
user.WvG1c1Td="qwerty"
So the "user.foobar" xattr is missing and there was a transaction abort
when unmounting the fs with the following traces in dmesg:
$ dmesg
[869800.159271] BTRFS warning (device sdi): access to eb bytenr 30474240 len 16384 out of range start 16015 len 25964
[869800.159293] ------------[ cut here ]------------
[869800.159296] WARNING: fs/btrfs/extent_io.c:4408 at report_eb_range+0x44/0x60 [btrfs], CPU#8: getfattr/2605179
[869800.168961] Modules linked in: btrfs dm_thin_pool (...)
[869800.190527] CPU: 8 UID: 0 PID: 2605179 Comm: getfattr Tainted: G W 7.3.0-rc3-btrfs-next-244+ #1 PREEMPT(full)
[869800.193726] Tainted: [W]=WARN
[869800.194502] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
[869800.197576] RIP: 0010:report_eb_range+0x44/0x60 [btrfs]
[869800.198985] Code: 48 8b 7b 18 (...)
[869800.203531] RSP: 0018:ffffce4541937d58 EFLAGS: 00010246
[869800.204609] RAX: 0000000000000000 RBX: ffff8dde054b8738 RCX: 0000000000000000
[869800.206137] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffffffffc04c92a0
[869800.207645] RBP: 0000000000003e8f R08: 0000000000000000 R09: 3fffffffffefffff
[869800.226175] R10: ffffce4541937a88 R11: 0000000000000003 R12: 000000000000656c
[869800.227594] R13: ffff8dde14be800f R14: 000000000000656c R15: 0000000000000069
[869800.229097] FS: 00007f59023bb780(0000) GS:ffff8de5788ed000(0000) knlGS:0000000000000000
[869800.231137] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[869800.232321] CR2: 0000558dc45e6a78 CR3: 0000000765b16004 CR4: 0000000000370ef0
[869800.233783] Call Trace:
[869800.234330] <TASK>
[869800.234791] read_extent_buffer+0x4d/0x100 [btrfs]
[869800.235898] btrfs_listxattr+0x199/0x240 [btrfs]
[869800.236912] vfs_listxattr+0x51/0xa0
[869800.237683] listxattr+0x7e/0x100
[869800.238398] path_listxattrat+0x9e/0x190
[869800.239117] do_syscall_64+0x89/0x470
[869800.239877] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[869800.240914] RIP: 0033:0x7f59024cdcb7
[869800.241686] Code: f0 ff ff 73 (...)
[869800.245353] RSP: 002b:00007ffd056dcdb8 EFLAGS: 00000246 ORIG_RAX: 00000000000000c2
[869800.246892] RAX: ffffffffffffffda RBX: 00007ffd056df2e2 RCX: 00007f59024cdcb7
[869800.248853] RDX: 0000000000006600 RSI: 0000558dc45e0470 RDI: 00007ffd056df2e2
[869800.250514] RBP: 00007ffd056df2e2 R08: 0000000000006600 R09: 0000000000006600
[869800.252295] R10: 0000000000000004 R11: 0000000000000246 R12: 00000000ffffff9c
[869800.254105] R13: 0000558dc45e0470 R14: 0000000000006600 R15: 0000000000000000
[869800.255931] </TASK>
[869800.256529] ---[ end trace 0000000000000000 ]---
[869800.260071] page: refcount:2 mapcount:0 mapping:000000007ccfc77f index:0x1d10 pfn:0x608c69
[869800.260075] memcg:ffff8dde00344d40
[869800.260076] aops:btree_aops [btrfs] ino:1
[869800.260151] flags: 0x17fffc00000402a(uptodate|lru|private|writeback|node=0|zone=2|lastcpupid=0x1ffff)
[869800.260154] raw: 017fffc00000402a fffff4adc773d4c8 fffff4adc48f3d88 ffff8de36504ba30
[869800.260155] raw: 0000000000001d10 ffff8dde054b8738 00000002ffffffff ffff8dde00344d40
[869800.260156] page dumped because: eb page dump
[869800.260157] BTRFS critical (device sdi): corrupt leaf: root=5 block=30474240 slot=5 ino=257, invalid location key type, have 46, expect 132 or 1
[869800.260161] BTRFS info (device sdi): leaf 30474240 gen 9 total ptrs 6 free space 15629 owner 5
[869800.260163] BTRFS info (device sdi): refs 3 lock_owner 0 current 2550294
[869800.260164] item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160
[869800.260165] inode generation 3 transid 0 size 0 nbytes 16384
[869800.260166] block group 0 mode 40755 links 1 uid 0 gid 0
[869800.260167] rdev 0 sequence 0 flags 0x0
[869800.260168] atime 1790340637.0
[869800.260169] ctime 1790340637.0
[869800.260169] mtime 1790340637.0
[869800.260170] otime 1790340637.0
[869800.260170] item 1 key (256 INODE_REF 256) itemoff 16111 itemsize 12
[869800.260172] index 0 name_len 2
[869800.260172] item 2 key (256 DIR_ITEM 982728850) itemoff 16073 itemsize 38
[869800.260173] location key (257 1 0) type 1
[869800.260174] transid 9 data_len 0 name_len 8
[869800.260175] item 3 key (257 INODE_ITEM 0) itemoff 15913 itemsize 160
[869800.260176] inode generation 9 transid 9 size 0 nbytes 0
[869800.260177] block group 0 mode 100664 links 1 uid 0 gid 0
[869800.260177] rdev 0 sequence 0 flags 0x0
[869800.260178] atime 1790340638.38778502
[869800.260179] ctime 1790340638.38778502
[869800.260179] mtime 1790340638.38778502
[869800.260180] otime 1790340638.38778502
[869800.260180] item 4 key (257 INODE_REF 256) itemoff 15895 itemsize 18
[869800.260181] index 2 name_len 8
[869800.260182] item 5 key (257 XATTR_ITEM 751495445) itemoff 15779 itemsize 116
[869800.260183] location key (0 0 0) type 8
[869800.266052] transid 9 data_len 6 name_len 13
[869800.266053] location key (8243121639454149888 46 7229457603934778967) type 0
[869800.266055] transid 113 data_len 26 name_len 0
[869800.266056] location key (8608196880778817904 120 162425) type 9
[869800.266057] transid 8391162079612502016 data_len 26982 name_len 25964
[869800.266058] BTRFS error (device sdi): block=30474240 write time tree block corruption detected
[869800.266091] ------------[ cut here ]------------
[869800.266092] WARNING: fs/btrfs/disk-io.c:336 at btree_csum_one_bio+0x20b/0x220 [btrfs], CPU#7: kworker/u50:7/2550294
[869800.268392] Modules linked in: btrfs dm_thin_pool (...)
[869800.365851] CPU: 7 UID: 0 PID: 2550294 Comm: kworker/u50:7 Tainted: G W 7.3.0-rc3-btrfs-next-244+ #1 PREEMPT(full)
[869800.368937] Tainted: [W]=WARN
[869800.369834] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org 04/01/2014
[869800.372778] Workqueue: writeback wb_workfn (flush-btrfs-3821)
[869800.374314] RIP: 0010:btree_csum_one_bio+0x20b/0x220 [btrfs]
[869800.375915] Code: 89 44 24 04 (...)
[869800.380639] RSP: 0018:ffffce4548e3f7d0 EFLAGS: 00010246
[869800.382008] RAX: 0000000000000000 RBX: ffff8dde054b8738 RCX: 0000000000000000
[869800.383850] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff8de091c2ddc0
[869800.385710] RBP: ffff8dde196a2000 R08: 0000000000000000 R09: 3fffffffffefffff
[869800.387388] R10: ffffce4548e3f500 R11: 0000000000000003 R12: ffffce4548e3f7d8
[869800.388973] R13: ffff8dde196a2000 R14: ffff8de36504b750 R15: ffff8dde4c497b00
[869800.390414] FS: 0000000000000000(0000) GS:ffff8de5788ad000(0000) knlGS:0000000000000000
[869800.392002] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[869800.393160] CR2: 000055cd6e92ad5c CR3: 00000007cb264001 CR4: 0000000000370ef0
[869800.394595] Call Trace:
[869800.395113] <TASK>
[869800.395570] btrfs_submit_bbio+0x872/0x890 [btrfs]
[869800.397284] write_meta_extent_buffer+0x70/0x80 [btrfs]
[869800.398940] btree_writepages+0x141/0x4f0 [btrfs]
[869800.400426] ? get_random_u32+0x8a/0xf0
[869800.401417] ? build_slab_freelist+0x47/0x130
[869800.402574] ? preempt_count_add+0x6b/0xa0
[869800.403633] ? _raw_spin_lock_irqsave+0x23/0x50
[869800.404807] ? _raw_spin_unlock_irqrestore+0x22/0x40
[869800.406085] ? alloc_from_new_slab+0x18f/0x330
[869800.407223] do_writepages+0xc6/0x160
[869800.408191] ? refill_objects+0xd8/0x300
[869800.409211] __writeback_single_inode+0x42/0x350
[869800.410408] writeback_sb_inodes+0x231/0x560
[869800.411511] wb_writeback+0x8a/0x300
[869800.412440] wb_workfn+0xbf/0x460
[869800.413291] ? _raw_spin_unlock+0x14/0x30
[869800.414328] ? finish_task_switch.isra.0+0xb9/0x380
[869800.415105] process_one_work+0x1d1/0x3d0
[869800.416633] worker_thread+0x1c4/0x330
[869800.417467] ? __pfx_worker_thread+0x10/0x10
[869800.418452] kthread+0xfc/0x130
[869800.419257] ? __pfx_kthread+0x10/0x10
[869800.420089] ret_from_fork+0x1f7/0x2c0
[869800.420863] ? __pfx_kthread+0x10/0x10
[869800.421654] ret_from_fork_asm+0x1a/0x30
[869800.422484] </TASK>
[869800.422953] ---[ end trace 0000000000000000 ]---
[869800.424005] BTRFS error (device sdi state A): Transaction 9 aborted (-EIO)
[869800.424010] BTRFS: error (device sdi state A) in __btrfs_run_delayed_items:1162: errno=-5 IO failure
[869800.424011] BTRFS info (device sdi state EA): forced readonly
[869800.424013] BTRFS warning (device sdi state EA): Skipping commit of aborted transaction.
[869800.424014] BTRFS: error (device sdi state EA) in cleanup_transaction:2076: errno=-5 IO failure
Fix this by always setting all fields in the new btrfs_dir_item when we
replace an existing xattr.
Fixes: 5f5bc6b1e2d5 ("Btrfs: make xattr replace operations atomic")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Returning -EAGAIN while leaving btrfs_uring_encoded_data in the cmd PDU
leaks if the request is cancelled or the ring exits before reissue.
io_uring does not free driver PDU allocations on cleanup.
Write: io_queue_sqe() always issues with IO_URING_F_NONBLOCK first, so
return -EAGAIN before allocating and free data on every exit.
Read: free on nowait -EAGAIN too; only -EIOCBQUEUED keeps the allocation
for btrfs_uring_read_finished().
Fixes: 34310c442e17 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)")
Fixes: e32dcdb0af9f ("btrfs: add io_uring interface for encoded writes")
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_uring_read_extent() runs only after btrfs_encoded_read() has taken
the inode shared lock and the extent lock. On failure it used to unlock
in out_fail, and a pages-array allocation failure returned -ENOMEM
without unlocking at all.
Unlock in the caller instead on all failure returns, matching the
copy_to_user() error path. The deferred -EIOCBQUEUED path still unlocks
in btrfs_uring_read_finished().
Fixes: 34310c442e17 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)")
Suggested-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
After btrfs_uring_read_extent(), the caller always jumped to out_acct.
That skips kfree(data->iov), which is only correct for -EIOCBQUEUED
where the deferred path owns the iov. On failure, fall through to
out_free instead.
Fixes: 34310c442e17 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)")
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If all bios finish before btrfs_encoded_read_regular_fill_pages()
returns, it calls btrfs_uring_read_extent_endio() and previously
returned the I/O status. A negative errno then made
btrfs_uring_read_extent() unlock and free while
btrfs_uring_read_finished() did the same again.
Return -EIOCBQUEUED so only the deferred path cleans up.
Reported-by: Yue Sun <samsun1006219@gmail.com>
Closes: https://lore.kernel.org/linux-btrfs/20260630091609.3414-1-samsun1006219@gmail.com/
Suggested-by: Jens Axboe <axboe@kernel.dk>
Fixes: 34310c442e17 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)")
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When paths_from_inode() fails, scrub_print_warning_inode() jumps to err
without dropping the reference taken by btrfs_get_fs_root(), leaking a
reference to the root every time path resolution fails while printing
scrub warnings. Every other error and success path of the function
drops the reference.
Drop the reference on the paths_from_inode() failure path too.
Fixes: 558540c17771 ("btrfs scrub: print paths of corrupted files")
CC: stable@vger.kernel.org
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If we exit early because the transaction that last used the root already
matches the current transaction, we leave the BTRFS_ROOT_IN_TRANS_SETUP
bit set in the root (which we just set right before the exit). While this
does not cause any functional issue, it makes callers of
btrfs_record_root_in_trans() lock fs_info->reloc_mutex and call
record_root_in_trans() for nothing, causing unnecessary lock contention,
until one of them clears the bit in record_root_in_trans().
One caller of btrfs_record_root_in_trans() is start_transaction(), used to
start new transaction or joining an existing one, which is a hot path.
So clear BTRFS_ROOT_IN_TRANS_SETUP on early exit.
Assisted-by: LLM
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
__fsverity_get_info() calls rhashtable_lookup_fast(), which just uses
rcu_read_lock() and doesn't directly synchronize with
fsverity_remove_info().
For the same inode this isn't a problem: its fsverity_info is removed
only at inode eviction time or upon failure to enable verity, when the
inode no longer needs its fsverity_info and it will no longer be
accessed via that inode.
However, this is broken and can cause a use-after-free for concurrent
__fsverity_get_info() for *different* inodes. Those rely on following
fsverity_info::rhash_head in the rhashtable under rcu_read_lock() only.
They also use fsverity_info::inode to do the key comparison.
Fix this by RCU-delaying the freeing of 'struct fsverity_info' after
it's been removed from the rhashtable.
Fixes: f77f281b6118 ("fsverity: use a hashtable to find the fsverity_info")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Sandeep Dhavale <dhavale@google.com>
Link: https://patch.msgid.link/20261001171349.81454-1-ebiggers@kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
|
|
iterate_dir() takes the directory's i_rwsem shared and holds it across
->iterate_shared(). For a directory that never has an entry and is
never removed the lock keeps nothing still, it only orders every reader
and every writer of that inode behind each other.
For the directory of a nullfs instance that matters. The instance of
the initial mount namespace is the root of every empty mount namespace
and the private instance is the root of every kernel thread, so one
inode is shared across users who have nothing else in common. And a
reader can hold the lock for as long as it likes: back the getdents()
buffer with a mapping of a file on a FUSE mount of your own, let the
copy of "." and ".." fault and let the server wait. Queue an exclusive
taker behind it, a mkdir() in that directory goes through start_dirop()
before the read-only mount is reported, and from then on every lookup
that misses the dcache in that directory, every create and every mount
on it waits until the server answers. One user of an empty mount
namespace stalls all the others.
Add FOP_IMMUTABLE for the file operations of a directory that never
changes and is never removed and let iterate_dir() skip the lock for
it. The flag never changes for a file, ->f_pos is protected by
f_pos_lock since directories are FMODE_ATOMIC_POS, IS_DEADDIR can't be
set on such a directory and neither touch_atime() nor fsnotify take
i_rwsem. Set it on the nullfs directory. The placeholder directories of
libfs never have an entry either but their owners remove them, so they
keep the lock.
Fixes: 9d4e752a24f7 ("namespace: allow creating empty mount namespaces")
Cc: stable@vger.kernel.org # v7.1+
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-19-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Refuse flock() and POSIX locks on nullfs. Its one inode is the root of
every kernel thread and the following patches make it the directory
that stands in for an unmounted mount, so a lock taken through one such
directory would block the locks of every other holder and F_GETLK would
tell them the pid of the holder. Give the directory file operations of
its own: what libfs gives an empty directory plus ->lock and ->flock
that fail with ENOLCK, the way a filesystem without lock support does.
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-17-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Don't let nullfs be watched. fanotify refuses mount and filesystem marks
on SB_NOUSER superblocks but inode marks of inotify, fanotify and
dnotify go through. The one inode of knullfs is the root of every
kernel thread and the following patches make it reachable from
userspace as the directory that stands in for an unmounted mount. A
watch placed through one such directory would report the opens through
all the others, across users.
Add FS_DISALLOW_NOTIFY next to FS_DISALLOW_NOTIFY_PERM, refuse a mark on
any object of such a filesystem in fsnotify_add_mark_list() where every
backend ends up and set it for nullfs. There's nothing to watch on a
permanently empty and immutable filesystem.
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-16-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Mark the root of knullfs with dont_mount(). Nothing is ever mounted on
the root of a kernel thread and the following patches make that root
reachable from userspace, so say it on the dentry where it doesn't
depend on the mount being in no namespace. Let do_lock_mount() refuse
such a target before it takes the inode lock and namespace_sem. The
flag is sticky so the check needs no lock. The one under the locks
stays for a mountpoint that is being removed.
Make the mount read-only as well. Its one inode is immutable so nothing
could be changed through it anyway, but MNT_READONLY makes that visible
the usual way: EROFS instead of EPERM and ST_RDONLY in statvfs().
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-15-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
The private nullfs instance that kernel threads are confined to is only
ever reachable through init_task's root and pwd. The following patches
point mounts at it as well, so keep it in a global.
No functional changes.
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-14-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
may_decode_fh() lets a caller who is privileged over the mount
namespace of @root decode handles below @root->dentry as long as the
mount is mounted and no locked child covers something below it. The
three parts are read one after the other without a lock: is_mounted()
and capable_wrt_mount() read ->mnt_ns and has_locked_children() walks
->mnt_mounts under a mount_lock of its own.
Today the gaps are harmless. A lazy umount in between clears ->mnt_ns,
but the locked children stay attached to their unmounted parent, so the
walk still finds them and the decode is refused either way. The
following patches detach every unmounted mount from its parent. Then
an umount between is_mounted() and the walk makes the walk come back
empty and the caller decodes into what a locked child covered.
So take mount_lock once and answer all three questions under it.
is_mounted() is stable there, umount_tree() clears ->mnt_ns on the
write side, and a mounted mount still has its locked children on its
list. ns_capable() under the spinlock is fine, generic_permission()
calls it in RCU walk already. has_locked_children() loses its locking
wrapper, its other callers hold namespace_sem or mount_lock anyway.
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-13-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
attach_recursive_mnt() transfers MNT_LOCKED from the top mount to the
mount that is moved beneath it with MOVE_MOUNT_BENEATH. This allows the
owner of a user namespace to replace its locked /proc or its root. The
mount beneath takes on the job of covering the underlying mountpoint
allowing the top mount to be unmounted.
Consider two mount namespaces:
(H) The host H has a shared mount P with a secret in P/d
(Z) Z is a user namespace made from M. Its copy of P receives
propagation from the host's P and its copy of M's cover on P/d is
locked Z's owner must not get to see P/d.
Now (H) mounts X on P/d. This propagates into (Z). The copy of X lands
beneath (Z)'s locked cover. The locked property is transfered from (Z)'s
cover to the copy of X propagated beneath it. The cover is now unlocked.
Now (H) unmounts X again. The copy of X in (Z) gets unmounted and the
covering mount is left unlocked on top of P/d. (Z) can now unmount it:
Z: umount2(P/d) = EINVAL /* the cover is locked */
H: mount X on P/d, umount X /* both propagate into Z /*
Z: umount2(P/d) = 0 /* the cover is now unlocked */
Z: read P/d/secret = "covered-by-root" /* secret revealed */
So only transfer the locked property to the mount beneath for mounts the
caller has placed. A propagated copy that lands beneath a locked mount
is locked as well so that the mount at the bottom of the stack carries a
lock the way every check expects. The mount on top of it remains locked
to ensure that it keeps covering even if the propagated mount is
unmounted again.
Fixes: c62a4766937e ("move_mount: transfer MNT_LOCKED")
Cc: stable@vger.kernel.org # v7.1+
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-9-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Locked mounts are special. They protect the underlying files and
directories from being revealed. do_umount() refuses to unmount locked
mounts but shrink_submounts() doesn't.
A shrinkable mount can become locked once the owner of a user namespace
puts it beneath a locked mount with MOVE_MOUNT_BENEATH. The locked
property now moves to the mount at the bottom making it possible to
unmount the top mount.
So umount() of an unlocked ancestor now expires the bottom mount first
and the covered directory is revealed:
move_mount(c -> x/hidden, MOVE_MOUNT_BENEATH) = 0
the cover after the lock moved: umount2(x/hidden) = 0
the holder of the lock: umount2(x/hidden) = EINVAL
the root of the copy, busy: umount2(x) = EBUSY
reads x/hidden/secret: "covered-by-root"
So leave a locked mount alone as it dies together with its parent. A
plain umount() of an unlocked mount with a locked one below it is EBUSY
from now on. It's the same for any other locked child. A lazy umount
still takes the whole tree.
mark_mounts_for_expiry() never sees a locked mount. lock_mnt_tree()
leaves a mount on an expiry list alone and nothing else puts a locked
one on such a list. Add an assert for this.
Fixes: 5ff9d8a65ce8 ("vfs: Lock in place mounts from more privileged users")
Fixes: c62a4766937e ("move_mount: transfer MNT_LOCKED")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-8-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
There's no point in updating access times of the nullfs instance. Raise
S_IMMUTABLE.
Fixes: 9d4e752a24f7 ("namespace: allow creating empty mount namespaces")
Cc: stable@vger.kernel.org # v7.1+
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-7-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
If mounts are propagated across user namespaces, attach_recursive_mnt()
locks every copy of the source mount to protect overmounts from
vanishing and revealing the underlying files or directories.
The user namespace is taken from the caller's mount namespaces since
this is where the mounts end up. Except, that's not always true.
Automounts may legitimately get popped in by tasks located in a
different mount and user namespace during path lookup.
Then check doesn't make sense at that point. The copy in the namespace
of the parent - which may be the host's - is now locked and the host
cannot change the flags of its own mounts anymore.
Here's the reproducer:
- host has debugfs mounted nosuid,nodev,noexec and shared
- tracefs automount below it is not yet active
- hand a child process a directory descriptor on that mount
- child process enters auser and mount namespace
- child process' namespace now holds a locked copy of the host's debugfs
mount that receives propagation from it
- child process stats "tracing/." through the descriptor
- lookup runs on the host's mount so the automount lands below the host's mount
- propagation puts a copy below the child's copy
- both try to clear the flags on the mount they got, with a bind remount:
host, on its own automount: MS_REMOUNT|MS_BIND = EPERM
child, on the copy in its namespace: MS_REMOUNT|MS_BIND = 0
So the lock landed on the host's mount instead of the child's copy.
Congrats. So we need to compare with the owner of the namespace the
mount actually gets mounted on. For all regular cases that is the
caller's mount namespace and so nothing changes.
Detached trees in anonymous mount namespaces by be handed over via
SCM_RIGHTS or inherited in other ways on purpose so the attaching task's
mount namespace is authoritative, not the creator of the detached tree.
Fixes: 132c94e31b8b ("vfs: Carefully propogate mounts across user namespaces")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-6-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
It's possible to add autmounts even when the parent mount isn't in the
mount namespace of the caller. The only requirement we have is that the
parent mount namespace must not be NULL, i.e., unmounted.
Problem is that clone_private_mount() has MNT_NS_INTERNAL which makes
that trivially true. So that passes the test and attach_recursive_mnt()
accepts that as a mount point and funny enough, count_mounts()
dereferences MNT_NS_INTERNAL. The problem is it is an error pointer...
So we can reach this in userspace via fanotify. A filesystem mark on the
lower filesystem of an overlay reports paths on the layer clone and
reading the event hands out a descriptor on it. For example with debugfs
as the lower layer it goes kaboom:
openat(evfd, "tracing", O_DIRECTORY)
Oops: general protection fault
KASAN: null-ptr-deref in range [0x1d0-0x1d7]
RIP: 0010:count_mounts+0x35/0x200
attach_recursive_mnt
finish_automount
And since that sleeping beauty happens under namespace_sem held for
writing every mount operation on the system blocks from then on.
Congrats.
Use is_mounted() instead which rejects unmounted and internal mounts
alike. The open fails with EINVAL just as it did before
clone_private_mount() used MNT_NS_INTERNAL.
Fixes: df820f8de4e4 ("ovl: make private mounts longterm")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-5-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
An immutable inode is never written to so write hints are pointless.
Refuse the hint on an immutable inode the way setattr(), fallocate() and
etxattr() refuse their changes.
Fixes: c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-3-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
In rcuwalk the dentry is validated before it is accepted. step_into()
step_into() rechecks d_seq and __follow_mount_rcu() rechecks mount_lock.
An entry that gets unlinked in between causes the lookup to retry and
miss.
A refwalk doesn't do this. lookup_fast() takes a reference on the hashed
dentry and simply accepts it. So an unlink that happens after the
reference was taken isn't seen by refwalk. That's fine for a simple
file. We just happened to open it before it was unlinked, no problem.
For a mountpoint and specifically a locked mountpoint it very much
isn't. The unlink detaches all mounts and then removes the name. Any
refwalk that hasn't traversed the mounts yet simply reveals the
underlying entry. It's a very narrow window but it can be hit:
reads of a covered file in 60 s, 3 walkers, ~15000 unlinks
no widening 27
that step + 200 us 3741
See the appended patch for a more reliable reproducer.
So check the entry after step_into(). unlink(), rmdir() and rename()
mark the dentry with dont_mount() before they detach the mounts and
remove the dentry.
So a dentry that is marked with DCACHE_CANT_MOUNT and is unhashed by the
time its mounts were looked at is a name that was unlinked under the
refwalk. DCACHE_CANT_MOUNT is read after the mounts were looked up and
it is set before they are detached by detach_mounts(). A refwalk that
missed the mounts will see DCACHE_CANT_MOUNT.
Plain d_unlinked() is fine. A rename takes the dentry off its hash chain
but ___d_drop() leaves d_hash.pprev set. So only __d_drop() and a rename
over the dentry unhash it. The only move that flips IS_ROOT splices in a
disconnected alias. That doesn't have DCACHE_CANT_MOUNT set.
Add the to step_into_slowpath(). ".." and LOOKUP_DOWN may legitimately
land on an unhashed directory. A refwalk that crossed onto a mount has
path.mnt different from nd->path.mnt. A dentry that a filesystem dropped
on its own (d_invalidate(), d_drop()) doesn't have the flag set and is
treated as before.
// SPDX-License-Identifier: GPL-2.0
/*
* unlink_covered: unlink a file that is a mountpoint in a detached copy of
* its mount, against walkers that open it through that copy.
*
* A is a tmpfs with the file f1 (content SECRET_F) and the plain file
* MARK_A. A' is an open_tree(OPEN_TREE_CLONE) copy of A with MARK_A bound
* on A'/f1, held through an O_PATH fd on its root once the tree fd is
* closed. Walkers open f1 through that fd while the driver unlinks A/f1,
* where nothing is mounted on it. A walker may read MARK_A or get ENOENT.
* A read of SECRET_F is a hit: the name was found after its mount was
* gone. The lockless walks use openat2(RESOLVE_CACHED).
*
* usage: unlink_covered [-t seconds] [-w walkers]
*/
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <linux/openat2.h>
#ifndef __NR_open_tree
#define __NR_open_tree 428
#endif
#ifndef __NR_move_mount
#define __NR_move_mount 429
#endif
#ifndef __NR_openat2
#define __NR_openat2 437
#endif
#ifndef OPEN_TREE_CLONE
#define OPEN_TREE_CLONE 1
#endif
#ifndef OPEN_TREE_CLOEXEC
#define OPEN_TREE_CLOEXEC O_CLOEXEC
#endif
#ifndef MOVE_MOUNT_F_EMPTY_PATH
#define MOVE_MOUNT_F_EMPTY_PATH 0x00000004
#endif
#define WORK "/tmp/uc"
#define ADIR WORK "/A"
static int duration = 60, nwalkers = 3;
static atomic_int stop, writer_waiting;
static pthread_rwlock_t cur_lock = PTHREAD_RWLOCK_INITIALIZER;
static int cur_fd = -1; /* the root of A', -1 while there is none */
static atomic_long n_unlink, n_walk, n_mark, n_enoent, n_other, n_secret,
n_secret_cached;
static void die(const char *what)
{
fprintf(stderr, "FATAL %s: %s\n", what, strerror(errno));
exit(2);
}
static void put_file(int dfd, const char *name, const char *content)
{
int fd = openat(dfd, name, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
0644);
if (fd < 0 || write(fd, content, strlen(content)) < 0)
die(name);
close(fd);
}
/* "plain/../" @n times, then f1: a longer walk that checks nothing on its way */
static char *longpath(int n)
{
char *p = malloc(n * 9 + 3), *q = p;
for (int i = 0; i < n; i++, q += 9)
memcpy(q, "plain/../", 9);
strcpy(q, "f1");
return p;
}
static void try_read(int dfd, const char *path, int cached)
{
struct open_how how = { .flags = O_RDONLY | O_CLOEXEC,
.resolve = RESOLVE_CACHED };
char buf[32] = "";
long n;
int fd;
if (cached)
fd = syscall(__NR_openat2, dfd, path, &how, sizeof(how));
else
fd = openat(dfd, path, O_RDONLY | O_CLOEXEC);
atomic_fetch_add(&n_walk, 1);
if (fd < 0) {
if (errno == ENOENT)
atomic_fetch_add(&n_enoent, 1);
else if (!cached || errno != EAGAIN)
atomic_fetch_add(&n_other, 1);
return;
}
n = read(fd, buf, sizeof(buf) - 1);
close(fd);
if (n >= 6 && !strncmp(buf, "SECRET", 6)) {
if (cached)
atomic_fetch_add(&n_secret_cached, 1);
if (!atomic_fetch_add(&n_secret, 1))
printf("HIT: read \"%s\" through %s (%s walk)\n", buf,
path, cached ? "lockless" : "any");
} else {
atomic_fetch_add(&n_mark, 1);
}
}
static void *walker(void *arg)
{
unsigned int r = (long)arg * 2654435761u;
while (!atomic_load(&stop)) {
char *p_long, *p_short;
int dfd;
while (atomic_load(&writer_waiting) && !atomic_load(&stop))
usleep(20);
pthread_rwlock_rdlock(&cur_lock);
dfd = cur_fd;
if (dfd < 0) {
pthread_rwlock_unlock(&cur_lock);
usleep(100);
continue;
}
r = r * 1103515245u + 12345u;
p_long = longpath(1 + (r >> 8) % 400);
p_short = longpath(0);
try_read(dfd, p_long, 0);
try_read(dfd, p_short, 0);
try_read(dfd, p_long, 1);
pthread_rwlock_unlock(&cur_lock);
free(p_long);
free(p_short);
}
return NULL;
}
/* hand the walkers a new A' (or none), close the old one */
static void publish(int fd)
{
int old;
atomic_fetch_add(&writer_waiting, 1);
pthread_rwlock_wrlock(&cur_lock);
old = cur_fd;
cur_fd = fd;
pthread_rwlock_unlock(&cur_lock);
atomic_fetch_sub(&writer_waiting, 1);
if (old >= 0)
close(old);
}
static void *driver(void *arg __attribute__((unused)))
{
int a;
if (mkdir(ADIR, 0755) && errno != EEXIST)
die("mkdir A");
if (mount("A", ADIR, "tmpfs", 0, "size=4M"))
die("mount A");
a = open(ADIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
if (a < 0 || mkdirat(a, "plain", 0755))
die("A/plain");
put_file(a, "MARK_A", "MARK_A");
while (!atomic_load(&stop)) {
int t, m, fd_a;
put_file(a, "f1", "SECRET_F");
t = syscall(__NR_open_tree, a, "",
OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC | AT_EMPTY_PATH);
if (t < 0)
die("open_tree A");
m = syscall(__NR_open_tree, a, "MARK_A",
OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
if (m < 0)
die("open_tree MARK_A");
if (syscall(__NR_move_mount, m, "", t, "f1",
MOVE_MOUNT_F_EMPTY_PATH))
die("move_mount");
close(m);
fd_a = openat(t, ".", O_PATH | O_DIRECTORY | O_CLOEXEC);
if (fd_a < 0)
die("open A'");
publish(fd_a);
usleep(200 + rand() % 1000);
close(t); /* A' is unmounted, fd_a holds it */
usleep(200 + rand() % 1000);
if (unlinkat(a, "f1", 0)) /* through A, a plain file there */
die("unlink f1");
atomic_fetch_add(&n_unlink, 1);
usleep(rand() % 300);
publish(-1);
}
close(a);
umount2(ADIR, MNT_DETACH);
return NULL;
}
int main(int argc, char **argv)
{
pthread_t d, *w;
int c, i;
setvbuf(stdout, NULL, _IOLBF, 0);
while ((c = getopt(argc, argv, "t:w:")) != -1) {
switch (c) {
case 't':
duration = atoi(optarg);
break;
case 'w':
nwalkers = atoi(optarg);
break;
default:
fprintf(stderr, "usage: unlink_covered [-t seconds] [-w walkers]\n");
return 2;
}
}
if (unshare(CLONE_NEWNS) ||
mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
die("unshare");
if (mkdir(WORK, 0755) && errno != EEXIST)
die("mkdir");
w = calloc(nwalkers, sizeof(*w));
for (i = 0; i < nwalkers; i++)
if (pthread_create(&w[i], NULL, walker, (void *)(long)i))
die("pthread_create");
if (pthread_create(&d, NULL, driver, NULL))
die("pthread_create");
sleep(duration);
atomic_store(&stop, 1);
pthread_join(d, NULL);
for (i = 0; i < nwalkers; i++)
pthread_join(w[i], NULL);
printf("unlink_covered: %d s, %d walkers: unlinks %ld walks %ld mark %ld enoent %ld other %ld SECRET %ld (lockless %ld)\n",
duration, nwalkers, atomic_load(&n_unlink), atomic_load(&n_walk),
atomic_load(&n_mark), atomic_load(&n_enoent),
atomic_load(&n_other), atomic_load(&n_secret),
atomic_load(&n_secret_cached));
return atomic_load(&n_secret) ? 1 : 0;
}
diff --git a/fs/namei.c b/fs/namei.c
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -35,6 +35,7 @@
#include <linux/fcntl.h>
#include <linux/device_cgroup.h>
#include <linux/fs_struct.h>
+#include <linux/delay.h>
#include <linux/posix_acl.h>
#include <linux/hash.h>
#include <linux/bitops.h>
@@ -1205,9 +1206,26 @@ static int sysctl_protected_symlinks __read_mostly;
static int sysctl_protected_hardlinks __read_mostly;
static int sysctl_protected_fifos __read_mostly;
static int sysctl_protected_regular __read_mostly;
+/* debug: widen the two windows of the detach_mounts() race */
+static int sysctl_detach_race_walk_us __read_mostly;
+static int sysctl_detach_race_unlink_us __read_mostly;
#ifdef CONFIG_SYSCTL
static const struct ctl_table namei_sysctls[] = {
+ {
+ .procname = "detach_race_walk_us",
+ .data = &sysctl_detach_race_walk_us,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
+ .procname = "detach_race_unlink_us",
+ .data = &sysctl_detach_race_unlink_us,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{
.procname = "protected_symlinks",
.data = &sysctl_protected_symlinks,
@@ -1878,6 +1896,10 @@ static struct dentry *lookup_fast(struct nameidata *nd)
dentry = __d_lookup(parent, &nd->last);
if (unlikely(!dentry))
return NULL;
+ /* debug: between finding a mountpoint and crossing its mounts */
+ if (unlikely(sysctl_detach_race_walk_us) && d_mountpoint(dentry))
+ usleep_range(sysctl_detach_race_walk_us,
+ sysctl_detach_race_walk_us + 10);
status = d_revalidate(nd->inode, &nd->last, dentry, nd->flags);
}
if (unlikely(status <= 0)) {
@@ -5693,6 +5715,10 @@ int vfs_unlink(struct mnt_idmap *idmap, struct inode *dir,
if (!error) {
dont_mount(dentry);
detach_mounts(dentry);
+ /* debug: the mounts are gone, d_delete() is still to come */
+ if (unlikely(sysctl_detach_race_unlink_us))
+ usleep_range(sysctl_detach_race_unlink_us,
+ sysctl_detach_race_unlink_us + 10);
}
}
}
Fixes: 8ed936b5671b ("vfs: Lazily remove mounts on unlinked files and directories.")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-2-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
unlink(), rmdir() and rename() remove the entry from the filesystem,
call detach_mounts() on the entry with the inode locked and then call
d_delete() once the inode is unlocked.
The thing is that between detach_mounts() and d_delete() the dentry is
still hashed and positive. But after detach_mounts() nothing covers the
dentry anymore. A lookup that finds this dentry in the dcache can
uncover what the mounts hid.
That's a problem when unlinking files or directories that are
mountpoints in other mount namespaces. Everybody who had the underlying
entry covered can race the detach_mount() call until d_delete() has run.
The race window isn't all that small. It encompasses namespace_unlock()
with a full synchronize_rcu_expedited() grace period and the
inode_unlock() of the entry.
In my experiments three walkers that kept trying read an overmounted
file in 11938 of the 16140 unlinks that removed its mountpoint from a
bind mount of the filesystem within a minute.
Fun fact, d_invalidate() has the same ordering problem but gets it
right. It unhashes the dentry first and detaches the mounts afterwards.
Let's do the same in __detach_mounts():
- A lookup that hasn't found the dentry yet misses it in the dcache and
waits for the directory lock that the caller holds until the name is
gone for good.
- A lockless lookup that found it already fails the mount_lock check
while the dentry still counts as a mountpoint and the d_seq check once
it doesn't, and retries.
This fixes the lockless path. We still need to fix the reference count
lookup in a follow-up patch.
So d_drop() the dentry. The dentry stays positive and held but can't be
found anymore. Then proceed with the detach and unlink.
Reproducer:
The reads were counted with the program below, three walkers for 60 s
in a VM with 4 CPUs. The race window is stretched with the debug patch
pasted here. fs.detach_race_walk_us sleeps in lookup_fast() once it
found a mountpoint dentry and before its mounts are crossed.
fs.detach_race_unlink_us sleeps in vfs_unlink() between detach_mounts()
and d_delete().
// SPDX-License-Identifier: GPL-2.0
/*
* unlink_covered: unlink a file that is a mountpoint in a detached copy of
* its mount, against lookups that open it through that copy.
*
* A is a tmpfs with the file f1 (content SECRET_F) and the plain file
* MARK_A. A' is an open_tree(OPEN_TREE_CLONE) copy of A with MARK_A bound
* on A'/f1, held through an O_PATH fd on its root once the tree fd is
* closed. Walkers open f1 through that fd while the driver unlinks A/f1,
* where nothing is mounted on it. A walker may read MARK_A or get ENOENT.
* A read of SECRET_F is a hit: the name was found after its mount was
* gone. The lockless walks use openat2(RESOLVE_CACHED).
*
* usage: unlink_covered [-t seconds] [-w walkers]
*/
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <linux/openat2.h>
#ifndef __NR_open_tree
#define __NR_open_tree 428
#endif
#ifndef __NR_move_mount
#define __NR_move_mount 429
#endif
#ifndef __NR_openat2
#define __NR_openat2 437
#endif
#ifndef OPEN_TREE_CLONE
#define OPEN_TREE_CLONE 1
#endif
#ifndef OPEN_TREE_CLOEXEC
#define OPEN_TREE_CLOEXEC O_CLOEXEC
#endif
#ifndef MOVE_MOUNT_F_EMPTY_PATH
#define MOVE_MOUNT_F_EMPTY_PATH 0x00000004
#endif
#define WORK "/tmp/uc"
#define ADIR WORK "/A"
static int duration = 60, nwalkers = 3;
static atomic_int stop, writer_waiting;
static pthread_rwlock_t cur_lock = PTHREAD_RWLOCK_INITIALIZER;
static int cur_fd = -1; /* the root of A', -1 while there is none */
static atomic_long n_unlink, n_walk, n_mark, n_enoent, n_other, n_secret,
n_secret_cached;
static void die(const char *what)
{
fprintf(stderr, "FATAL %s: %s\n", what, strerror(errno));
exit(2);
}
static void put_file(int dfd, const char *name, const char *content)
{
int fd = openat(dfd, name, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
0644);
if (fd < 0 || write(fd, content, strlen(content)) < 0)
die(name);
close(fd);
}
/* "plain/../" @n times, then f1: a longer walk that checks nothing on its way */
static char *longpath(int n)
{
char *p = malloc(n * 9 + 3), *q = p;
for (int i = 0; i < n; i++, q += 9)
memcpy(q, "plain/../", 9);
strcpy(q, "f1");
return p;
}
static void try_read(int dfd, const char *path, int cached)
{
struct open_how how = { .flags = O_RDONLY | O_CLOEXEC,
.resolve = RESOLVE_CACHED };
char buf[32] = "";
long n;
int fd;
if (cached)
fd = syscall(__NR_openat2, dfd, path, &how, sizeof(how));
else
fd = openat(dfd, path, O_RDONLY | O_CLOEXEC);
atomic_fetch_add(&n_walk, 1);
if (fd < 0) {
if (errno == ENOENT)
atomic_fetch_add(&n_enoent, 1);
else if (!cached || errno != EAGAIN)
atomic_fetch_add(&n_other, 1);
return;
}
n = read(fd, buf, sizeof(buf) - 1);
close(fd);
if (n >= 6 && !strncmp(buf, "SECRET", 6)) {
if (cached)
atomic_fetch_add(&n_secret_cached, 1);
if (!atomic_fetch_add(&n_secret, 1))
printf("HIT: read \"%s\" through %s (%s walk)\n", buf,
path, cached ? "lockless" : "any");
} else {
atomic_fetch_add(&n_mark, 1);
}
}
static void *walker(void *arg)
{
unsigned int r = (long)arg * 2654435761u;
while (!atomic_load(&stop)) {
char *p_long, *p_short;
int dfd;
while (atomic_load(&writer_waiting) && !atomic_load(&stop))
usleep(20);
pthread_rwlock_rdlock(&cur_lock);
dfd = cur_fd;
if (dfd < 0) {
pthread_rwlock_unlock(&cur_lock);
usleep(100);
continue;
}
r = r * 1103515245u + 12345u;
p_long = longpath(1 + (r >> 8) % 400);
p_short = longpath(0);
try_read(dfd, p_long, 0);
try_read(dfd, p_short, 0);
try_read(dfd, p_long, 1);
pthread_rwlock_unlock(&cur_lock);
free(p_long);
free(p_short);
}
return NULL;
}
/* hand the walkers a new A' (or none), close the old one */
static void publish(int fd)
{
int old;
atomic_fetch_add(&writer_waiting, 1);
pthread_rwlock_wrlock(&cur_lock);
old = cur_fd;
cur_fd = fd;
pthread_rwlock_unlock(&cur_lock);
atomic_fetch_sub(&writer_waiting, 1);
if (old >= 0)
close(old);
}
static void *driver(void *arg __attribute__((unused)))
{
int a;
if (mkdir(ADIR, 0755) && errno != EEXIST)
die("mkdir A");
if (mount("A", ADIR, "tmpfs", 0, "size=4M"))
die("mount A");
a = open(ADIR, O_PATH | O_DIRECTORY | O_CLOEXEC);
if (a < 0 || mkdirat(a, "plain", 0755))
die("A/plain");
put_file(a, "MARK_A", "MARK_A");
while (!atomic_load(&stop)) {
int t, m, fd_a;
put_file(a, "f1", "SECRET_F");
t = syscall(__NR_open_tree, a, "",
OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC | AT_EMPTY_PATH);
if (t < 0)
die("open_tree A");
m = syscall(__NR_open_tree, a, "MARK_A",
OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC);
if (m < 0)
die("open_tree MARK_A");
if (syscall(__NR_move_mount, m, "", t, "f1",
MOVE_MOUNT_F_EMPTY_PATH))
die("move_mount");
close(m);
fd_a = openat(t, ".", O_PATH | O_DIRECTORY | O_CLOEXEC);
if (fd_a < 0)
die("open A'");
publish(fd_a);
usleep(200 + rand() % 1000);
close(t); /* A' is unmounted, fd_a holds it */
usleep(200 + rand() % 1000);
if (unlinkat(a, "f1", 0)) /* through A, a plain file there */
die("unlink f1");
atomic_fetch_add(&n_unlink, 1);
usleep(rand() % 300);
publish(-1);
}
close(a);
umount2(ADIR, MNT_DETACH);
return NULL;
}
int main(int argc, char **argv)
{
pthread_t d, *w;
int c, i;
setvbuf(stdout, NULL, _IOLBF, 0);
while ((c = getopt(argc, argv, "t:w:")) != -1) {
switch (c) {
case 't':
duration = atoi(optarg);
break;
case 'w':
nwalkers = atoi(optarg);
break;
default:
fprintf(stderr, "usage: unlink_covered [-t seconds] [-w walkers]\n");
return 2;
}
}
if (unshare(CLONE_NEWNS) ||
mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
die("unshare");
if (mkdir(WORK, 0755) && errno != EEXIST)
die("mkdir");
w = calloc(nwalkers, sizeof(*w));
for (i = 0; i < nwalkers; i++)
if (pthread_create(&w[i], NULL, walker, (void *)(long)i))
die("pthread_create");
if (pthread_create(&d, NULL, driver, NULL))
die("pthread_create");
sleep(duration);
atomic_store(&stop, 1);
pthread_join(d, NULL);
for (i = 0; i < nwalkers; i++)
pthread_join(w[i], NULL);
printf("unlink_covered: %d s, %d walkers: unlinks %ld walks %ld mark %ld enoent %ld other %ld SECRET %ld (lockless %ld)\n",
duration, nwalkers, atomic_load(&n_unlink), atomic_load(&n_walk),
atomic_load(&n_mark), atomic_load(&n_enoent),
atomic_load(&n_other), atomic_load(&n_secret),
atomic_load(&n_secret_cached));
return atomic_load(&n_secret) ? 1 : 0;
}
diff --git a/fs/namei.c b/fs/namei.c
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -35,6 +35,7 @@
#include <linux/fcntl.h>
#include <linux/device_cgroup.h>
#include <linux/fs_struct.h>
+#include <linux/delay.h>
#include <linux/posix_acl.h>
#include <linux/hash.h>
#include <linux/bitops.h>
@@ -1205,9 +1206,26 @@ static int sysctl_protected_symlinks __read_mostly;
static int sysctl_protected_hardlinks __read_mostly;
static int sysctl_protected_fifos __read_mostly;
static int sysctl_protected_regular __read_mostly;
+/* debug: widen the two windows of the detach_mounts() race */
+static int sysctl_detach_race_walk_us __read_mostly;
+static int sysctl_detach_race_unlink_us __read_mostly;
#ifdef CONFIG_SYSCTL
static const struct ctl_table namei_sysctls[] = {
+ {
+ .procname = "detach_race_walk_us",
+ .data = &sysctl_detach_race_walk_us,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
+ .procname = "detach_race_unlink_us",
+ .data = &sysctl_detach_race_unlink_us,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{
.procname = "protected_symlinks",
.data = &sysctl_protected_symlinks,
@@ -1878,6 +1896,10 @@ static struct dentry *lookup_fast(struct nameidata *nd)
dentry = __d_lookup(parent, &nd->last);
if (unlikely(!dentry))
return NULL;
+ /* debug: between finding a mountpoint and crossing its mounts */
+ if (unlikely(sysctl_detach_race_walk_us) && d_mountpoint(dentry))
+ usleep_range(sysctl_detach_race_walk_us,
+ sysctl_detach_race_walk_us + 10);
status = d_revalidate(nd->inode, &nd->last, dentry, nd->flags);
}
if (unlikely(status <= 0)) {
@@ -5693,6 +5715,10 @@ int vfs_unlink(struct mnt_idmap *idmap, struct inode *dir,
if (!error) {
dont_mount(dentry);
detach_mounts(dentry);
+ /* debug: the mounts are gone, d_delete() is still to come */
+ if (unlikely(sysctl_detach_race_unlink_us))
+ usleep_range(sysctl_detach_race_unlink_us,
+ sysctl_detach_race_unlink_us + 10);
}
}
}
Fixes: 8ed936b5671b ("vfs: Lazily remove mounts on unlinked files and directories.")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261002-work-mount-fixes-4-v1-1-dd44b89d44ce@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Pull smb client fixes from Paulo Alcantara:
"Fix a series of data corruption and I/O error bugs found by running
generic/363 (fsx) in a loop against Windows Server 2022 and Samba.
- Stop data dirtied past EOF through an mmap from reappearing as file
content once the file is extended by a write, truncate, zero range,
copy range or clone range
- Flush dirty data and drain in-flight I/O before operations that
assume the pagecache and the server agree on the file: querying
allocated ranges, the O_TRUNC open, interior zero range, and
server-side copy/clone
- Stop a genuine size-extending zero range or preallocate from being
refused with -EOPNOTSUPP when the inode is not read caching, by
querying the server's authoritative EOF instead of trusting a stale
cached i_size
- Zero the untransferred tail of a short read, both in the netfs
read-gaps path (where stale folio content could otherwise be
written back to the server) and in the DIO/unbuffered read
collector, and tell a real EOF apart from a stale cached
remote_i_size after a lease downgrade
- Require stable pages on signed connections so a buffered write
can't modify a folio whose signature has already been computed and
is in flight, which the server rejected with STATUS_ACCESS_DENIED
and the client surfaced as -EIO
- Split several cifsFileInfo flags out of a shared bitfield byte so
concurrent updates taken under different locks no longer clobber
each other through a byte-level RMW"
* tag 'cifs-fixes-7.3-rc6' of https://git.manguebit.org/linux:
smb: client: split cifsFileInfo bitfields to avoid shared-byte RMW races
smb: client: require stable pages for signed connections
smb: client: distinguish real EOF from a stale remote_i_size on read
netfs: zero the tail of a short DIO/unbuffered read
smb: client: only require read lease for size-extending preallocate
netfs: zero gaps in read-gaps folio to avoid writing back stale data
smb: client: only require read lease for size-extending zero range
smb: client: drain and invalidate before server-side copy/clone
smb: client: flush dirty data before zeroing a range
smb: client: drain outstanding I/O before truncating on O_TRUNC open
smb: client: flush and commit data before querying allocated ranges
smb: client: discard post-EOF pagecache when extending a file via clone range
smb: client: discard post-EOF pagecache when extending a file via copy range
smb: client: discard post-EOF pagecache when extending a file via zero range
smb: client: clear post-EOF pagecache when extending a file via truncate
netfs: clear post-EOF pagecache when extending a file via write
|
|
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: 35f96de04127 ("bpf: Introduce BPF token object")
Cc: stable@vger.kernel.org # v6.9+
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-17-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: 1064f874abc0 ("mnt: Tuck mounts under others instead of creating shadow/side mounts.")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-15-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: bf630c401641 ("vfs: add notifications for mount attach and detach")
Cc: stable@vger.kernel.org # v6.15+
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-13-be34c83956ae@kernel.org
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Commit 02587a4af82a ("fs: refuse fspick() on internal superblocks")
blocked fspick() on SB_NOUSER superblocks. But that's not the only way
to reconfigure_super(). mount(MS_REMOUNT) and umount() of the caller's
root without MNT_DETACH. The root of an empty mount namespace is a
nullfs mount and all mount namespaces share that superblock:
nullfs: fspick: FAIL errno=22 (Invalid argument)
nullfs: mount(MS_REMOUNT|MS_RDONLY): ok(0)
nullfs after remount: statfs(/): magic=0x4e554c4c flags=0x21 RDONLY
nullfs: umount2("/", 0): ok(0)
Refuse both like fspick() does. Only root in the initial user namespace
can do this and nullfs is empty and immutable so the flags don't buy
anything. But they show up in statfs() for the root of every mount
namespace on the host.
Fixes: 9d4e752a24f7 ("namespace: allow creating empty mount namespaces")
Cc: stable@vger.kernel.org # v7.1+
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-11-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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 bb4609405752 ("statmount: read the parent of an
unmounted mount under mount_lock") fixed for statmount().
Check that both mounts are in the caller's namespace before their
parents are read. Every path returns EINVAL either way.
Fixes: e0c9c0afd2fc ("mnt: Update detach_mounts to leave mounts connected")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-10-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: 96f5d2e05165 ("attach_recursive_mnt(): unify the mnt_change_mountpoint() logics")
Cc: stable@vger.kernel.org # v6.17+
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-8-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: 9b8a0ba68246 ("mount: add OPEN_TREE_NAMESPACE")
Cc: stable@vger.kernel.org # v7.0+
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-6-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: e149ed2b805f ("take the targets of /proc/*/ns/* symlinks to separate fs")
Fixes: ef4144ac2dec ("pidfs: allow bind-mounts")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-4-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: 1064f874abc0 ("mnt: Tuck mounts under others instead of creating shadow/side mounts.")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-2-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
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: bf630c401641 ("vfs: add notifications for mount attach and detach")
Cc: stable@vger.kernel.org # v6.15+
Link: https://patch.msgid.link/20260930-work-mount-fixes-3-v1-1-be34c83956ae@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
The invalidHandle, swapfile, oplock_break_cancelled, offload,
and status_file_deleted fields are stored in the same bitfield byte
in struct cifsFileInfo, but are updated in different code paths that
may run simultaneously, and are protected by different locks. Since
bitfield assignments generate byte-level read-modify-write operations,
a modification to one flag can overwrite a concurrent modification to
another flag.
To avoid these races, convert these flags from a bitfield to
separate bool fields.
Closes: https://lore.kernel.org/r/7689764e-c0f6-4016-9557-b54cf4a3de4e@redhat.com
Fixes: 3bc303c254335 ("cifs: convert oplock breaks to use slow_work facility (try #4)")
Fixes: 4e8aea30f7751 ("smb3: enable swap on SMB3 mounts")
Fixes: ffceb7640cbfe ("smb: client: do not defer close open handles to deleted files")
Fixes: 173217bd73365 ("smb3: retrying on failed server close")
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
When signing, cifs computes the SMB signature over the pagecache folios
in place and then hands those same folios to the socket. If a buffered
write mutates a folio while a write subrequest is still in flight, the
signature no longer matches the data that follows it, the server rejects
the write with STATUS_ACCESS_DENIED (-EACCES), and the error is latched
in the mapping, so the next fsync()/fallocate() returns -EIO.
Mark the mapping for stable writes so netfs_perform_write() waits for
writeback to complete before modifying an in-flight folio. This is
only needed when the connection is signed.
Fixes: 3ee1a1fc3981 ("cifs: Cut over to using netfslib")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
smb2_readv_callback() sets NETFS_SREQ_HIT_EOF whenever a short read
lines up with netfs_read_remote_i_size(inode), the server's EOF as the
client currently believes it. That belief can be stale: after a lease
downgrade and handle reopen, the tracked remote_i_size can sit below
the client's own i_size while an extending write hasn't reached the
server yet. A read in that gap comes back short for a reason that has
nothing to do with the file's real size, but was still marked HIT_EOF,
and netfs reports a short read for it as-is.
Only treat it as real EOF when the position is also at or past the
client's own i_size; otherwise mark it NETFS_SREQ_CLEAR_TAIL instead,
which tells netfs the shortfall is safe to zero-fill rather than
report as a short read.
This is what fsx (generic/363) sees as "short read: 0x0 bytes instead
of 0x<n>" against a Windows server.
Fixes: 1da29f2c39b6 ("netfs, cifs: Fix handling of short DIO read")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
The buffered read collector zero-fills the tail of a short read that
stops below the inode's i_size (netfs_clear_unread()), so a read that
races an extending write still returns the expected number of bytes.
The non-buffered collector path does no such thing: it just records
how much was transferred.
Add the same zero-fill for the non-buffered case, gated on
NETFS_SREQ_CLEAR_TAIL: a subreq's source sets that flag when a short
result from it is known to be safe to treat as a hole, as opposed to
NETFS_SREQ_HIT_EOF, which means the read genuinely ran off the end of
the file and should be reported short as-is. Only CLEAR_TAIL should
zero-fill here; a real EOF must stay a real short read.
No source currently sets CLEAR_TAIL on an unbuffered/DIO subrequest,
so this is inert on its own -- a following change teaches cifs to set
it in the one case that needs it.
Fixes: e2d46f2ec332 ("netfs: Change the read result collector to only use one work item")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
smb3_simple_falloc() refuses any fallocate that clears FALLOC_FL_KEEP_SIZE
with -EOPNOTSUPP whenever the inode is not read caching:
/* if file not oplocked can't be sure whether asking to extend size */
if (!CIFS_CACHE_READ(cifsi))
if (!keep_size) {
...
return rc;
}
As with smb3_zero_range(), the read lease is only needed to trust the
cached size when deciding whether the request extends the file. When it
is not held, the size can instead be fetched from the server, which is
authoritative, rather than refusing the request outright: after
flushing, query the server's end of file and take the larger of it and
the cached size for the interior-vs-extend decision. The larger of the
two is used because the server's end of file reflects another client's
growth while the cached size reflects this client's own writes that may
not have reached the server yet; using the server size alone would
wrongly treat an interior request as extending when a range flush left
an extending write unwritten.
smb3_simple_fallocate_range(), which performs the actual interior
emulation, decided whether the range already lies past EOF from its own
i_size_read(inode) rather than the old_eof computed above. In the
leaseless case, that is exactly the stale, undershooting size this
patch works around: a genuinely interior range can read as past EOF by
that stale count, which skips the FSCTL_QUERY_ALLOCATED_RANGES check
entirely and overwrites already-allocated server data with zeroes
instead of only filling the holes. Pass old_eof into
smb3_simple_fallocate_range() and use it for that comparison instead of
re-deriving a second, inconsistent one.
Rejecting interior requests is observed as generic/363 randomly failing
against Windows Server with
do_preallocate: fallocate: Operation not supported
fsx issues an interior, non-KEEP_SIZE preallocate while the inode is
transiently not read caching: the server had just downgraded the file's
lease from RWH to RH after breaking the write caching, and the ensuing
handle reopen/revalidation left CIFS_CACHE_READ momentarily clear. The
range sat within the server's end of file, so no extend was needed, yet
it was refused and fsx aborted. This keeps the emulation correct even
when a genuine lease break from another client leaves the inode
without read caching -- the case the -EOPNOTSUPP guard turned into a
hard failure. The extra flush and round trip only happen when both
keep_size is false and no read lease is held; every other case is
unchanged.
Fixes: 9ccf3216238c ("Add support for original fallocate")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
netfs_read_gaps() leaves gaps around a streaming write's dirty region
unread on the server side, so a short/EOF response there left stale
folio content that later got written back.
Zero it after the read completes, not before: netfs_wait_for_read()
returns rreq->transferred once ret >= 0, so build an iterator over the
same bvec array the read used, advance past ret, and zero the rest.
The dirty region already maps to sink pages in that array rather than
the real folio, so this can't touch it, and only the actual shortfall
gets zeroed instead of the whole gap upfront.
Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
smb3_zero_range() refuses any FALLOC_FL_ZERO_RANGE that clears
FALLOC_FL_KEEP_SIZE with -EOPNOTSUPP whenever the inode is not read
caching:
/* if file not oplocked can't be sure whether asking to extend size */
rc = -EOPNOTSUPP;
if (keep_size == false && !CIFS_CACHE_READ(cifsi))
goto zero_range_exit;
The read lease is only needed to trust the cached i_size when deciding
whether the range extends the file. When it is not held, the size can
instead be fetched from the server, which is authoritative, rather than
refusing the request outright: query the server's end of file and take
the larger of it and the cached size for the interior-vs-extend
decision. The larger of the two is used because the server's end of
file reflects another client's growth while the cached size reflects
this client's own writes that may not have reached the server yet;
using the server size alone would wrongly shrink the file when the
range flush above left an extending write unwritten.
The query isn't lease-protected either, so its result is only trusted to
confirm the range is interior, never to justify extending: if it still
shows the range going past EOF, refuse with -EOPNOTSUPP instead of
calling SMB2_set_eof(), which could otherwise shrink the file if another
client extended it further between the query and the call. For the same
reason, take the larger of the already-computed i_size and a fresh
i_size_read(inode) right before that call, instead of relying on either
alone: the local variable carries the query's result, which is never
written back to the inode, while a concurrent write on this client can
still extend the cached i_size during the flush, the query, or the
zero-data round trip that happen in between.
Rejecting interior ranges is observed as generic/363 randomly failing
against Windows Server with
do_zero_range: fallocate: Operation not supported
fsx issues an interior, non-KEEP_SIZE zero range while the inode is
transiently not read caching: the server had just downgraded the file's
lease from RWH to RH after breaking the write caching, and the ensuing
handle reopen/revalidation left CIFS_CACHE_READ momentarily clear. The
range sat well within the server's end of file, so no extend was
needed, yet the range was refused and fsx aborted. This keeps the
emulation correct even when a genuine lease break from another client
leaves the inode without read caching -- the case the -EOPNOTSUPP guard
turned into a hard failure. The extra round trip only happens on the
no-lease path; the common cached case is unchanged.
Fixes: 30175628bf7f ("[SMB3] Enable fallocate -z support for SMB3 mounts")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
cifs_file_copychunk_range() and the clone (FICLONE) path of
cifs_remap_file_range() do not serialise the destination page cache
against the server-side copy the way the other server-side range
operations (smb3_zero_range(), smb3_punch_hole(), smb3_collapse_range()
and smb3_insert_range()) do.
cifs_file_copychunk_range() invalidates the destination range with
filemap_invalidate_inode(), which takes and drops the mapping's
invalidate_lock internally, so the lock is no longer held when the
copychunk ioctl is issued. It also never drains in-flight netfs I/O on
the target. The clone path never takes the invalidate_lock at all (only
i_rwsem), uses a bare truncate_inode_pages_range() and likewise does not
drain outstanding I/O.
As a result an asynchronous destination writeback can complete after the
server-side copy/clone has run and reinstate stale data over the region
just written by the server, corrupting the file. This is the same class
of corruption as commit d7d2adcd022b ("smb/client: flush dirty data
before punching a hole") and has been seen randomly in generic/363
against Windows Server.
Fix both paths to follow the established ordering: hold the target
mapping's invalidate_lock across the flush and invalidation of the
destination and the server ioctl, and call netfs_wait_for_outstanding_io()
on the target to drain in-flight writes before the ioctl is issued. Only
the target inode's invalidate_lock is required, as the source is merely
flushed and not invalidated; i_rwsem (already held via
lock_two_nondirectories()) is acquired before the invalidate_lock,
matching the VFS lock ordering. Since filemap_invalidate_inode() takes
that same lock internally, replace it with its own unmap/flush/invalidate
steps instead of calling it, and return early on a zero-length copy to
avoid a range underflow.
Fixes: 8101d6e112e2 ("cifs: Fix copy offload to flush destination region")
Fixes: c54fc3a4f375 ("cifs: Fix flushing, invalidation and file size with FICLONE")
Reviewed-by: David Howells <dhowells@redhat.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
