Skip to content

Commit

Permalink
FEXLoader: Fixes newer wine versions and Fedora
Browse files Browse the repository at this point in the history
This was brought up by #3831 but I finally got the courage to look at
the hard problem.

Although I'm only tackling half of the problem with this PR, which is
that FEXLoader needs to strip the rootfs path from the executed path if
it begins with the rootfs, plus some changes to the surrounding code.

The primary concern here is that when an application has been executed
under FEX, specifically through binfmt_misc, then FEX needs to prepend
the full rootfs path otherwise Linux can't find the program.
Additionally execveat with an FD will resolve a full path to the rootfs.

So past FEX's initial setup, we need to strip off the rootfs path to
provide an "absolute" path that is visible to the guest application
later. Which is kind of funny since we have a `RootFSRedirect` function
which did the exact opposite. This was due to legacy problems in the
original ELFLoader that couldn't handle symlinks correctly, which has
since been resolved, so that no longer needs to exist.

There was also some weirdness in `GetApplicationNames` where the passed
in argument list was modifying Args[0] and then saving the Program as
well. Which I just got rid of. Also stopped passing in the arguments by
value because....why did I write it like that?

In InterpreterHandler we now need to check if we can open the path
inside the rootfs or fallback without it. Plus I had to change the
shebang handling so it stopped prefixing the rootfs AGAIN. Took the time
to change the shebang handling there so it stops creating string copies
and instead just generates views. Which again this would be nice if it
could use `std::ranges::split_view` but since that would raise the clang
minspec to 16, we have to write it ourselves.

Overall this fixes a fairly major flaw with how we were representing
`/proc/self` to the application, which was breaking wine since it would
prefix the rootfs multiple times, which was weird.

It doesn't address the remaining problem in #3831, which is that
applications can still see some of the leaky abstractions with symlinks
through the rootfs, but I want to get at least this step in.
  • Loading branch information
Sonicadvance1 committed Sep 22, 2024
1 parent c0b8a0d commit 231b3a0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 37 deletions.
5 changes: 2 additions & 3 deletions Source/Common/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInte
return Program;
}

ApplicationNames GetApplicationNames(fextl::vector<fextl::string> Args, bool ExecFDInterp, int ProgramFDFromEnv) {
ApplicationNames GetApplicationNames(const fextl::vector<fextl::string>& Args, bool ExecFDInterp, int ProgramFDFromEnv) {
if (Args.empty()) {
// Early exit if we weren't passed an argument
return {};
Expand All @@ -346,8 +346,7 @@ ApplicationNames GetApplicationNames(fextl::vector<fextl::string> Args, bool Exe
fextl::string Program {};
fextl::string ProgramName {};

Args[0] = RecoverGuestProgramFilename(std::move(Args[0]), ExecFDInterp, ProgramFDFromEnv);
Program = Args[0];
Program = RecoverGuestProgramFilename(std::move(Args[0]), ExecFDInterp, ProgramFDFromEnv);

bool Wine = false;
for (size_t CurrentProgramNameIndex = 0; CurrentProgramNameIndex < Args.size(); ++CurrentProgramNameIndex) {
Expand Down
2 changes: 1 addition & 1 deletion Source/Common/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct PortableInformation {
*
* @return The application name and path structure
*/
ApplicationNames GetApplicationNames(fextl::vector<fextl::string> Args, bool ExecFDInterp, int ProgramFDFromEnv);
ApplicationNames GetApplicationNames(const fextl::vector<fextl::string>& Args, bool ExecFDInterp, int ProgramFDFromEnv);

/**
* @brief Loads the FEX and application configurations for the application that is getting ready to run.
Expand Down
69 changes: 36 additions & 33 deletions Source/Tools/FEXLoader/FEXLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,17 @@ class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriter {
};
} // namespace AOTIR

void InterpreterHandler(fextl::string* Filename, const fextl::string& RootFS, fextl::vector<fextl::string>* args) {
// Open the Filename to determine if it is a shebang file.
int FD = open(Filename->c_str(), O_RDONLY | O_CLOEXEC);
bool InterpreterHandler(fextl::string* Filename, const fextl::string& RootFS, fextl::vector<fextl::string>* args) {
int FD {-1};

// Attempt to open the filename from the rootfs first.
FD = open(fextl::fmt::format("{}{}", RootFS, *Filename).c_str(), O_RDONLY | O_CLOEXEC);
if (FD == -1) {
return;
// Failing that, attempt to open the filename directly.
FD = open(Filename->c_str(), O_RDONLY | O_CLOEXEC);
if (FD == -1) {
return false;
}
}

std::array<char, 257> Header;
Expand All @@ -150,47 +156,40 @@ void InterpreterHandler(fextl::string* Filename, const fextl::string& RootFS, fe
// Is the file large enough for shebang
if (ReadSize <= 2) {
close(FD);
return;
return true;
}

// Handle shebang files
if (Data[0] == '#' && Data[1] == '!') {
fextl::string InterpreterLine {Data.begin() + 2, // strip off "#!" prefix
std::find(Data.begin(), Data.end(), '\n')};
fextl::vector<fextl::string> ShebangArguments {};
std::string_view InterpreterLine {Data.begin() + 2, // strip off "#!" prefix
std::find(Data.begin(), Data.end(), '\n')};
fextl::vector<std::string_view> ShebangArguments {};

// Shebang line can have a single argument
fextl::istringstream InterpreterSS(InterpreterLine);
fextl::string Argument;
while (std::getline(InterpreterSS, Argument, ' ')) {
if (Argument.empty()) {
continue;
using namespace std::literals;
auto Begin = InterpreterLine.begin();
auto ArgEnd = Begin;
auto End = InterpreterLine.end();
const auto Delim = " "sv;
for (; ArgEnd != End && Begin != End; Begin = ArgEnd + 1) {
// Find the end
ArgEnd = std::find_first_of(Begin, End, Delim.begin(), Delim.end());
if (Begin != ArgEnd) {
const auto View = std::string_view(Begin, ArgEnd - Begin);
if (!View.empty()) {
ShebangArguments.emplace_back(View);
}
}
ShebangArguments.push_back(std::move(Argument));
}

// Executable argument
fextl::string& ShebangProgram = ShebangArguments[0];

// If the filename is absolute then prepend the rootfs
// If it is relative then don't append the rootfs
if (ShebangProgram[0] == '/') {
ShebangProgram = RootFS + ShebangProgram;
}
*Filename = ShebangProgram;
*Filename = ShebangArguments[0];

// Insert all the arguments at the start
args->insert(args->begin(), ShebangArguments.begin(), ShebangArguments.end());
}
close(FD);
}

void RootFSRedirect(fextl::string* Filename, const fextl::string& RootFS) {
auto RootFSLink = ELFCodeLoader::ResolveRootfsFile(*Filename, RootFS);

if (FHU::Filesystem::Exists(RootFSLink)) {
*Filename = RootFSLink;
}
return true;
}

FEX::Config::PortableInformation ReadPortabilityInformation() {
Expand Down Expand Up @@ -404,10 +403,14 @@ int main(int argc, char** argv, char** const envp) {
FEXCore::Profiler::Init();
FEXCore::Telemetry::Initialize();

RootFSRedirect(&Program.ProgramPath, LDPath());
InterpreterHandler(&Program.ProgramPath, LDPath(), &Args);
// Program.ProgramPath is highly likely to be prefixed with FEX's rootfs. Get rid of that.
if (Program.ProgramPath.starts_with(LDPath()) == 0) {
Program.ProgramPath = Program.ProgramPath.substr(LDPath().size());
}

bool ProgramExists = InterpreterHandler(&Program.ProgramPath, LDPath(), &Args);

if (!ExecutedWithFD && FEXFD == -1 && !FHU::Filesystem::Exists(Program.ProgramPath)) {
if (!ExecutedWithFD && FEXFD == -1 && !ProgramExists) {
// Early exit if the program passed in doesn't exist
// Will prevent a crash later
fextl::fmt::print(stderr, "{}: command not found\n", Program.ProgramPath);
Expand Down

0 comments on commit 231b3a0

Please sign in to comment.