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

Linus Torvalds
2024-01-24T17:11:18Z

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

On Wed, 24 Jan 2024 at 08:54, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Hmm. That whole thing is disgusting. I think it should have checked
> FMODE_EXEC, and I have no idea why it doesn't.

Maybe because FMODE_EXEC gets set for uselib() calls too? I dunno. I
think it would be even better if we had the 'intent' flags from
'struct open_flags' available, but they aren't there in the
file_open() security chain.

Anyway, moving current->in_execve earlier looks fairly trivial, but I
worry about the randomness. I'd be *so*( much happier if this crazy
flag went away, and it got changed to look at the open intent instead.

Attached patch is ENTIRELY UNTESTED. And disgusting.

I went back and looked. This whole disgusting thing goes back to 2009
and commit f9ce1f1cda8b ("Add in_execve flag into task_struct").

Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1795 bytes --]

fs/exec.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8cdd5b2dd09c..fc1d6befe830 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1843,7 +1843,6 @@ static int bprm_execve(struct linux_binprm *bprm)
* where setuid-ness is evaluated.
*/
check_unsafe_exec(bprm);
- current->in_execve = 1;
sched_mm_cid_before_execve(current);

sched_exec();
@@ -1860,7 +1859,6 @@ static int bprm_execve(struct linux_binprm *bprm)
sched_mm_cid_after_execve(current);
/* execve succeeded */
current->fs->in_exec = 0;
- current->in_execve = 0;
rseq_execve(current);
user_events_execve(current);
acct_update_integrals(current);
@@ -1879,7 +1877,6 @@ static int bprm_execve(struct linux_binprm *bprm)

sched_mm_cid_after_execve(current);
current->fs->in_exec = 0;
- current->in_execve = 0;

return retval;
}
@@ -1910,6 +1907,7 @@ static int do_execveat_common(int fd, struct filename *filename,
/* We're below the limit (still or again), so we don't want to make
* further execve() calls fail. */
current->flags &= ~PF_NPROC_EXCEEDED;
+ current->in_execve = 1;

bprm = alloc_bprm(fd, filename, flags);
if (IS_ERR(bprm)) {
@@ -1965,6 +1963,7 @@ static int do_execveat_common(int fd, struct filename *filename,
free_bprm(bprm);

out_ret:
+ current->in_execve = 0;
putname(filename);
return retval;
}
@@ -1985,6 +1984,7 @@ int kernel_execve(const char *kernel_filename,
if (IS_ERR(filename))
return PTR_ERR(filename);

+ current->in_execve = 1;
bprm = alloc_bprm(fd, filename, 0);
if (IS_ERR(bprm)) {
retval = PTR_ERR(bprm);
@@ -2024,6 +2024,7 @@ int kernel_execve(const char *kernel_filename,
out_free:
free_bprm(bprm);
out_ret:
+ current->in_execve = 0;
putname(filename);
return retval;
}