linux-kernel - Re: [PATCH v4 28/30] audit,landlock: Add AUDIT_EXE_LANDLOCK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG48ez3oTpoVH=en8yAwS2u=kuyh8rKqPQFjDCe_Muh7N9E_Tg@mail.gmail.com>
Date: Mon, 13 Jan 2025 15:55:42 +0100
From: Jann Horn <jannh@...gle.com>
To: Mickaël Salaün <mic@...ikod.net>, 
	Christian Brauner <brauner@...nel.org>, Al Viro <viro@...iv.linux.org.uk>
Cc: Eric Paris <eparis@...hat.com>, Paul Moore <paul@...l-moore.com>, 
	Günther Noack <gnoack@...gle.com>, 
	"Serge E . Hallyn" <serge@...lyn.com>, Ben Scarlato <akhna@...gle.com>, 
	Casey Schaufler <casey@...aufler-ca.com>, Charles Zaffery <czaffery@...lox.com>, 
	Daniel Burgener <dburgener@...ux.microsoft.com>, 
	Francis Laniel <flaniel@...ux.microsoft.com>, James Morris <jmorris@...ei.org>, 
	Jeff Xu <jeffxu@...gle.com>, Jorge Lucangeli Obes <jorgelo@...gle.com>, Kees Cook <kees@...nel.org>, 
	Konstantin Meskhidze <konstantin.meskhidze@...wei.com>, Matt Bobrowski <mattbobrowski@...gle.com>, 
	Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>, Phil Sutter <phil@....cc>, 
	Praveen K Paladugu <prapal@...ux.microsoft.com>, Robert Salvet <robert.salvet@...lox.com>, 
	Shervin Oloumi <enlightened@...gle.com>, Song Liu <song@...nel.org>, 
	Tahera Fahimi <fahimitahera@...il.com>, Tyler Hicks <code@...icks.com>, audit@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v4 28/30] audit,landlock: Add AUDIT_EXE_LANDLOCK_DENY rule type

+Christian and Al Viro to double-check what I'm saying

On Wed, Jan 8, 2025 at 4:44 PM Mickaël Salaün <mic@...ikod.net> wrote:
> -static const void *get_current_exe(const char **path_str, size_t *path_size)
> +static const void *get_current_exe(const char **path_str, size_t *path_size,
> +                                  struct inode **inode)
>  {
>         struct mm_struct *mm = current->mm;
>         struct file *file __free(fput) = NULL;
> @@ -93,6 +96,8 @@ static const void *get_current_exe(const char **path_str, size_t *path_size)
>
>         *path_size = size;
>         *path_str = path;
> +       ihold(file_inode(file));
> +       *inode = file_inode(file);
>         return no_free_ptr(buffer);
>  }

This looks unsafe: Once the reference to the file has been dropped
(which happens implicitly on return from get_current_exe()), nothing
holds a reference on the mount point or superblock anymore (the file
was previously holding a reference to the mount point through
->f_path.mnt), and so the superblock can be torn down and freed. But
the reference to the inode lives longer and is only cleaned up on
return from the caller get_current_details().

So I think this code can hit the error check for "Busy inodes after
unmount" in generic_shutdown_super(), which indicates that in theory,
use-after-free can occur.

For context, here are two older kernel security issues that also
involved superblock UAF due to assuming that it's possible to just
hold refcounted references to inodes:

https://project-zero.issues.chromium.org/42451116
https://project-zero.issues.chromium.org/379667898

For fixing this, one option would be to copy the entire "struct path"
(which holds references on both the mount point and the inode) instead
of just copying the inode pointer.