Fix program_invocation_short_name when argv[0] has no '/'#3972
Open
User-green wants to merge 1 commit into
Open
Fix program_invocation_short_name when argv[0] has no '/'#3972User-green wants to merge 1 commit into
User-green wants to merge 1 commit into
Conversation
my_program_invocation_short_name (and my___progname) were computed as strrchr(box64->argv[0], '/') + 1. When argv[0] contains no '/', strrchr returns NULL, and NULL + 1 yields (char*)1 — an invalid pointer. Guest code that reads program_invocation_short_name / __progname then dereferences it and crashes (e.g. a std::string ctor doing strlen((char*)1)). This triggers when a process is exec'd with a bare argv[0] (no path), such as a renamed fork-server child, and shows up as a SIGSEGV in the forked child that does not reproduce when argv[0] is a path. Match glibc: with no '/', the short name is the whole argv[0]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WARNING!
The whole thing was done by claude, I only tested! It works in my given case and might be usefull - but it also might be harmfull on some deep level I dont understand.
I went ahead and created this PR because it would be better than just opening an issue while sitting on a fix.
Here is what Claude gave to me:
The bug in one sentence
box64 computes the emulated
program_invocation_short_nameasstrrchr(argv[0], '/') + 1, which isNULL + 1 == (char*)1(a wild pointer)whenever
argv[0]contains no/.Where
src/wrapped/wrappedlibc.c, in the libcCUSTOM_INITmacro (~line 4928):my_program_invocation_name(full) is fine; only the short name is wrong.Why it's wrong
glibc's
program_invocation_short_nameis the basename ofargv[0]: the partafter the last
/, or the whole string when there is no/. The box64 codeomits the "no slash" case, so it returns
(char*)1instead ofargv[0].Impact
Any emulated program that reads
program_invocation_short_nameor__prognameand uses it as a string (logging, error prefixes,
std::string, etc.)dereferences
(char*)1and crashes — but only whenargv[0]has no/.When
argv[0]has no/in practice"adb").argv[0](has/, works), the renamed child has a bareargv[0](crashes).It is not actually fork-specific and not dynarec-specific.
Reproducer (deterministic)
x86_64 Android
adbunder box64:adbspawns a fork-server child renamed toadb. liblog's__android_log_set_default_tagbuilds astd::stringfromprogram_invocation_short_name->strlen((char*)1)-> SIGSEGV.adb version(no fork/rename) is fine.
BOX64_DYNAREC=0crashes identically (rules outdynarec).
Evidence (how it was confirmed)
Interpreter-mode build under gdb on the box64 core:
strlenwith first argRDI = 0x1.__cxa_guard_release->__android_log_set_default_tag->std::__1::basic_string::basic_string(char const*)-> the wrappedstrlen.my_program_invocation_short_name == 0x1, whilemy_program_invocation_namepoints to a valid"adb".The fix
With no
/, useargv[0]itself (matches glibc):(Calling
strrchrtwice is fine; this runs once at init. A temp variable wouldavoid the double call if preferred.)
Verification done
Built current
main(commit c018889) from source as v0.4.3 on aarch64(Cortex-A55, Arch Linux ARM, kernel 6.18), with this one-line change.
adb start-serverSIGSEGVs in the forked child (dynarec on AND off).adb start-server-> "daemon started successfully", server answersadb version(dynarec on AND off).Related (worth mentioning, out of scope for the 1-line PR)
The 32-bit path
src/wrapped32/wrappedlibc.chas a different but adjacentmistake:
strrchr(box64->argv[0], '/')with no+ 1— that keeps theleading
/when present, and isNULLwhen absent. Not reproduced here; left out to keep the PR minimal. Could be fixed the same way.Closes #3971.