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: <20250113-zeitraffer-gewachsen-fa9a69868c70@brauner>
Date: Mon, 13 Jan 2025 16:02:43 +0100
From: Christian Brauner <brauner@...nel.org>
To: Jann Horn <jannh@...gle.com>
Cc: Mickaël Salaün <mic@...ikod.net>, 
	Al Viro <viro@...iv.linux.org.uk>, 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

On Mon, Jan 13, 2025 at 03:55:42PM +0100, Jann Horn wrote:
> +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

s/looks/is/g

> (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.

Yep, it sure would.

> 
> 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.

path_get() indeed