Re: [PATCH] landlock: Fix superblock use-after-free in release_inode()

["Mickaël Salaün" <mic@digikod.net>]

On Tue, Oct 06, 2026 at 12:58:47PM +0200, Günther Noack wrote:
> 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: cb2c7d1a1776 ("landlock: Support filesystem access-control")
> Assisted-by: LLM
> Signed-off-by: Günther Noack <gnoack@google.com>

Thanks! It's in my next branch.

> ---
>  security/landlock/fs.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index cab43892ec2f..0959e9f487e6 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -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 = {
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
> 
>