[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a9210754-2f94-4075-872f-8f6a18f4af07@I-love.SAKURA.ne.jp>
Date: Thu, 25 Jan 2024 23:16:42 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Kees Cook <keescook@...omium.org>,
John Johansen <john.johansen@...onical.com>,
Paul Moore <paul@...l-moore.com>
Cc: Kevin Locke <kevin@...inlocke.name>,
Josh Triplett
<josh@...htriplett.org>,
Mateusz Guzik <mjguzik@...il.com>, Al Viro <viro@...iv.linux.org.uk>,
linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org,
Kentaro Takeda <takedakn@...data.co.jp>
Subject: Re: [6.8-rc1 Regression] Unable to exec apparmor_parser from
virt-aa-helper
On 2024/01/25 3:27, Linus Torvalds wrote:
> The whole cred use of current->in_execve in tomoyo should
> *also* be fixed, but I didn't even try to follow what it actually
> wanted.
Due to TOMOYO's unique domain transition (transits to new domain before
execve() succeeds and returns to old domain if execve() failed), TOMOYO
depends on a tricky ordering shown below.
----------
// a caller tries execve().
sys_execve() {
do_execve() {
do_execveat_common() {
bprm_execve() {
prepare_bprm_creds() {
prepare_exec_creds() {
prepare_creds() {
security_prepare_creds() {
tomoyo_cred_prepare() {
if (s->old_domain_info && !current->in_execve) { // false because s->old_domain_info == NULL.
s->domain_info = s->old_domain_info;
s->old_domain_info = NULL;
}
}
}
}
}
}
current->in_execve = 1;
do_open_execat() {
(...snipped...)
security_file_open() {
tomoyo_file_open() // Not checked because current->in_execve == 1.
}
(...snipped...)
}
exec_binprm() {
search_binary_handler() {
security_bprm_check() {
tomoyo_bprm_check_security() {
if (!s->old_domain_info) { // true because s->old_domain_info == NULL.
tomoyo_find_next_domain() {
// Checks execute permission here.
s->old_domain_info = s->domain_info; // Remember old domain.
s->domain_info = domain; // Transit to new domain.
}
}
}
}
fmt->load_binary() { // e.g. load_script() in fs/binfmt_script.c
open_exec() {
// Not checked because current->in_execve == 1.
}
}
}
search_binary_handler() {
security_bprm_check() {
tomoyo_bprm_check_security() {
if (!s->old_domain_info) { // false because s->old_domain_info != NULL.
} else {
// Checks read permission here.
}
}
}
}
// An error happens after s->domain_info was updated.
}
current->in_execve = 0;
// No chance to restore s->domain_info.
}
}
}
// returning an error code to the caller.
}
// the caller retries execve().
sys_execve() {
do_execve() {
do_execveat_common() {
bprm_execve() {
prepare_bprm_creds() {
prepare_exec_creds() {
prepare_creds() {
security_prepare_creds() {
tomoyo_cred_prepare() {
if (s->old_domain_info && !current->in_execve) { // true because s->old_domain_info != NULL && current->in_execve == 0.
s->domain_info = s->old_domain_info; // Transit to old domain.
s->old_domain_info = NULL;
}
}
}
}
}
}
current->in_execve = 1;
do_open_execat() {
(...snipped...)
security_file_open() {
tomoyo_file_open() // Not checked because current->in_execve == 1.
}
(...snipped...)
}
exec_binprm() {
search_binary_handler() {
security_bprm_check() {
tomoyo_bprm_check_security() {
if (!s->old_domain_info) { // true because s->old_domain_info == NULL.
tomoyo_find_next_domain() {
// Checks execute permission here.
s->old_domain_info = s->domain_info; // Remember old domain.
s->domain_info = domain; // Transit to new domain.
}
}
}
}
fmt->load_binary() { // e.g. load_script() in fs/binfmt_script.c
open_exec() {
// Not checked because current->in_execve == 1.
}
}
}
search_binary_handler() {
security_bprm_check() {
tomoyo_bprm_check_security() {
if (!s->old_domain_info) { // false because s->old_domain_info != NULL.
} else {
// Checks read permission here.
}
}
}
}
fmt->load_binary() { // e.g. load_elf_binary() in fs/binfmt_elf.c
begin_new_exec() {
security_bprm_committed_creds() {
tomoyo_bprm_committed_creds() {
s->old_domain_info = NULL; // Forget old domain.
}
}
}
}
}
current->in_execve = 0;
}
}
}
}
----------
Commit 978ffcbf00d8 ("execve: open the executable file before doing anything else")
broke the ordering and commit 4759ff71f23e ("exec: Check __FMODE_EXEC instead of
in_execve for LSMs") and commit 3eab830189d9 ("uselib: remove use of __FMODE_EXEC")
fixed the regression.
But current->in_execve remains required unless an LSM callback that is called when
an execve() request failed which existed as security_bprm_free() until Linux 2.6.28
revives...
Powered by blists - more mailing lists