Re: [6.8-rc1 Regression] Unable to exec apparmor_parser from virt-aa-helper

Tetsuo Handa
2024-01-25T14:17:55Z

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