mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-10-26 10:57:38 +08:00
landlock: Fix superblock use-after-free in release_inode()
Without this fix, release_inode() (task 1) can race with a umount
operation in hook_sb_delete() (task 2). The order of operations in
which the race occurs is:
* task 2: hook_sb_delete() arrives at the end of the function, waiting
for landlock_superblock(sb)->inode_refs to become zero.
* task 1: release_inode(): does atomic_long_dec_and_test(), decrementing
inode_refs to zero.
* task 2: gets woken up through some other means, sees that inode_refs
is zero and returns. The caller is the unmount operation and proceeds
to release the superblock.
* task 1: Runs wake_up_var(&landlock_superblock(sb)->inode_refs),
*dereferencing the sb_security on the freed-up superblock*.
Notably, wake_up_var() does not dereference the pointer passed to it,
and only uses it to determine an overapproximated set of tasks to wake
up. So while it's potentially possible for an attacker to provide a
different value for that pointer, this only results in the wrong set of
tasks getting woken up, and in this scenario, no further tasks are
actually waiting.
Nevertheless, KASAN detects it as a slab-use-after-free read on
sb->s_security:
BUG: KASAN: slab-use-after-free in release_inode+0x196/0x290
Read of size 8 at addr ffff888103b6c0a8 by task repro/217
release_inode+0x196/0x290
landlock_put_object+0x71/0x90
landlock_free_rules+0x38/0xc0
landlock_put_ruleset+0x47/0x130
fop_ruleset_release+0x33/0x40
__fput+0x305/0x780
Freed by task 54:
kfree+0x11c/0x390
process_scheduled_works+0x766/0xe50
worker_thread+0x7bc/0xb20
The fix is to determine &landlock_superblock(inode->i_sb)->inode_refs only once
earlier in release_inode(), and later use that precalculated address for the
wake_up_var() call, without having to dereference through the superblock.
Cc: stable@vger.kernel.org
Fixes: cb2c7d1a17 ("landlock: Support filesystem access-control")
Assisted-by: LLM
Signed-off-by: Günther Noack <gnoack@google.com>
Link: https://patch.msgid.link/20261006105847.2541190-1-gnoack@google.com
Signed-off-by: Mickaël Salaün <mic@digikod.net>
This commit is contained in:
1 parent
8e4c736cff
commit
3e271a27c4
1 file changed
+5
-5
@@ -61,7 +61,7 @@ static void release_inode(struct landlock_object *const object)
|
||||
__releases(object->lock)
|
||||
{
|
||||
struct inode *const inode = object->underobj;
|
||||
struct super_block *sb;
|
||||
atomic_long_t *inode_refs;
|
||||
|
||||
if (!inode) {
|
||||
spin_unlock(&object->lock);
|
||||
@@ -77,8 +77,8 @@ static void release_inode(struct landlock_object *const object)
|
||||
* Makes sure that if the filesystem is concurrently unmounted,
|
||||
* hook_sb_delete() will wait for us to finish iput().
|
||||
*/
|
||||
sb = inode->i_sb;
|
||||
atomic_long_inc(&landlock_superblock(sb)->inode_refs);
|
||||
inode_refs = &landlock_superblock(inode->i_sb)->inode_refs;
|
||||
atomic_long_inc(inode_refs);
|
||||
spin_unlock(&object->lock);
|
||||
/*
|
||||
* Because object->underobj was not NULL, hook_sb_delete() and
|
||||
@@ -92,8 +92,8 @@ static void release_inode(struct landlock_object *const object)
|
||||
*/
|
||||
|
||||
iput(inode);
|
||||
if (atomic_long_dec_and_test(&landlock_superblock(sb)->inode_refs))
|
||||
wake_up_var(&landlock_superblock(sb)->inode_refs);
|
||||
if (atomic_long_dec_and_test(inode_refs))
|
||||
wake_up_var(inode_refs);
|
||||
}
|
||||
|
||||
static const struct landlock_object_underops landlock_fs_underops = {
|
||||
|
||||
Reference in new issue
Block a user