linux-kernel - Re: [PATCH 2/2] openat2: add OA2_INHERIT

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: <20240425-akrobatisch-palmen-4c1e7a0f69d2@brauner>
Date: Thu, 25 Apr 2024 16:02:46 +0200
From: Christian Brauner <brauner@...nel.org>
To: Stas Sergeev <stsp2@...dex.ru>
Cc: linux-kernel@...r.kernel.org, Stefan Metzmacher <metze@...ba.org>, 
	Eric Biederman <ebiederm@...ssion.com>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Andy Lutomirski <luto@...nel.org>, Jan Kara <jack@...e.cz>, Jeff Layton <jlayton@...nel.org>, 
	Chuck Lever <chuck.lever@...cle.com>, Alexander Aring <alex.aring@...il.com>, 
	David Laight <David.Laight@...lab.com>, linux-fsdevel@...r.kernel.org, linux-api@...r.kernel.org, 
	Paolo Bonzini <pbonzini@...hat.com>, Christian Göttsche <cgzones@...glemail.com>
Subject: Re: [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag

>  struct open_flags {
> -	int open_flag;
> +	u64 open_flag;

Btw, this change taken together with

> +#define VALID_OPENAT2_FLAGS (VALID_OPEN_FLAGS | OA2_INHERIT_CRED)

is also ripe to causes subtle bugs and security issues. This new
VALID_OPENAT2_FLAGS define bypasses

	BUILD_BUG_ON_MSG(upper_32_bits(VALID_OPEN_FLAGS),
			 "struct open_flags doesn't yet handle flags > 32 bits");

in build_open_flags(). And right now lookup_open(), open_last_lookups(),
and do_open() just do:

	int open_flag = op->open_flag;

Because op->open_flag was 32bit that was fine. But now ->open_flag is
64bit which means we truncate the upper 32bit including OA2_INHERIT_CRED
or any other new flag in the upper 32bits in those functions.

So as soon as there's an additional check in e.g., do_open() for
OA2_INHERIT_CRED or in any of the other helpers that's security relevant
we're fscked because that flag is never seen and no bot will help us
here. And it's super easy to miss during review...