fix(linux): prevent infinite re-exec loop under AppArmor#2239
Conversation
The `MaybeReenterWithoutASLR()` function in `src/benchmark.cc` caused infinite execv() loops when running benchmarks under AppArmor-enabled LSMs. The existing fix from google#1985 only checked whether personality(ADDR_NO_RANDOMIZE) succeeded in the current process before calling execv(). However, some LSMs like AppArmor can silently reset personality flags during the execve() system call transition, even though the flag was successfully set in the parent process. Signed-off-by: anish <anishesg@users.noreply.github.com>
| // Build new argv with test argument (allocate on heap to survive exec) | ||
| char** new_argv = static_cast<char**>( | ||
| malloc(sizeof(char*) * (arg_count + 2))); | ||
| if (!new_argv) _exit(1); | ||
|
|
||
| for (int i = 0; i < arg_count; ++i) { | ||
| new_argv[i] = argv[i]; | ||
| } | ||
|
|
||
| // Add test argument with pipe write fd (allocate on heap) | ||
| char* test_arg = static_cast<char*>(malloc(64)); | ||
| if (!test_arg) _exit(1); | ||
| std::snprintf(test_arg, 64, | ||
| "--benchmark_aslr_test_child=%d", pipefd[1]); | ||
| new_argv[arg_count] = test_arg; | ||
| new_argv[arg_count + 1] = nullptr; | ||
|
|
||
| execv(argv[0], new_argv); |
There was a problem hiding this comment.
I haven't seen if ASAN splats on this,
but i really don't think this is allowed.
Can you even touch heap in a child?
Certainly you can't alloc before calling execv?
There was a problem hiding this comment.
good catch, switched to stack allocation which is safe before exec
There was a problem hiding this comment.
I'm not sure any (either stack or heap) is safe before calling execv.
It would only be safe if it copies the actual underlying strings, does it do that?
I'm hoping ASAN will answer this. If this isn't safe, we need to produce new_argv before fork().
|
Thank you for taking a look! This does roughly look how i imagined it would look. |
| // Count existing arguments | ||
| int arg_count = 0; | ||
| while (argv[arg_count] != nullptr) ++arg_count; | ||
|
|
||
| // Build new argv with test argument (allocate on heap to survive exec) | ||
| char** new_argv = static_cast<char**>( | ||
| malloc(sizeof(char*) * (arg_count + 2))); | ||
| if (!new_argv) _exit(1); | ||
|
|
||
| for (int i = 0; i < arg_count; ++i) { | ||
| new_argv[i] = argv[i]; | ||
| } |
There was a problem hiding this comment.
I'm not sure there is a need to preserve existing arguments,
just the {argv[0], <magic>, nullptr} is enough.
There was a problem hiding this comment.
simplified to just {argv[0], magic, nullptr}
There was a problem hiding this comment.
Err, i didn't actually mean a stack-based array, just that it only needs to have 3 elements.
(in principle, i'm not sure we need to pass argv[0], either)
…eap alloc Signed-off-by: anish <anishesg@users.noreply.github.com>
|
I haven't personally reproduced the original issue reported in #2184, but the reporter confirmed the infinite loop occurs on Ubuntu 24.04 with AppArmor enabled. The fork-based verification approach should be safe - if the personality doesn't survive exec, we just skip re-exec and continue normally. |
| char result = ((test_personality != -1) && | ||
| (internal::get_as_unsigned(test_personality) & ADDR_NO_RANDOMIZE)) | ||
| ? 1 : 0; | ||
| (void)write(write_fd, &result, 1); |
There was a problem hiding this comment.
Looks like GCC wants some better check than cast-to-void here.
2 participants
The
MaybeReenterWithoutASLR()function insrc/benchmark.cccaused infinite execv() loops when running benchmarks under AppArmor-enabled LSMs. The existing fix from #1985 only checked whether personality(ADDR_NO_RANDOMIZE) succeeded in the current process before calling execv(). However, some LSMs like AppArmor can silently reset personality flags during the execve() system call transition, even though the flag was successfully set in the parent process.This fix implements a fork-based verification: before committing to re-exec the parent process, we fork a test child, exec it with a special argument, and have it report back whether ADDR_NO_RANDOMIZE actually survived the exec boundary. Only if the child confirms the flag persists do we proceed with re-executing the parent. This prevents infinite loops in environments where the LSM strips the personality flag during exec.
The test child is invoked with
--benchmark_aslr_test_child=<fd>, checks its personality after exec, writes the result back through the provided file descriptor, and exits. This approach maintains hermeticity by using process arguments rather than environment variables.Fixes #2184