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

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- 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;
 }