From 78f7090a21fae15182e5c8cdfc2ce59a5563b907 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sun, 22 Sep 2024 14:50:43 -0700 Subject: [PATCH] FEXLoader: Fixes newer wine versions and Fedora 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. 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. --- Source/Common/Config.cpp | 5 +- Source/Common/Config.h | 2 +- Source/Tools/FEXLoader/FEXLoader.cpp | 69 +++++++++++++++------------- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/Source/Common/Config.cpp b/Source/Common/Config.cpp index ed84c37daf..622f9942ab 100644 --- a/Source/Common/Config.cpp +++ b/Source/Common/Config.cpp @@ -337,7 +337,7 @@ fextl::string RecoverGuestProgramFilename(fextl::string Program, bool ExecFDInte return Program; } -ApplicationNames GetApplicationNames(fextl::vector Args, bool ExecFDInterp, int ProgramFDFromEnv) { +ApplicationNames GetApplicationNames(const fextl::vector& Args, bool ExecFDInterp, int ProgramFDFromEnv) { if (Args.empty()) { // Early exit if we weren't passed an argument return {}; @@ -346,8 +346,7 @@ ApplicationNames GetApplicationNames(fextl::vector Args, bool Exe fextl::string Program {}; fextl::string ProgramName {}; - Args[0] = RecoverGuestProgramFilename(std::move(Args[0]), ExecFDInterp, ProgramFDFromEnv); - Program = Args[0]; + Program = RecoverGuestProgramFilename(Args[0], ExecFDInterp, ProgramFDFromEnv); bool Wine = false; for (size_t CurrentProgramNameIndex = 0; CurrentProgramNameIndex < Args.size(); ++CurrentProgramNameIndex) { diff --git a/Source/Common/Config.h b/Source/Common/Config.h index c6b37fab8a..5c13c210a7 100644 --- a/Source/Common/Config.h +++ b/Source/Common/Config.h @@ -40,7 +40,7 @@ struct PortableInformation { * * @return The application name and path structure */ -ApplicationNames GetApplicationNames(fextl::vector Args, bool ExecFDInterp, int ProgramFDFromEnv); +ApplicationNames GetApplicationNames(const fextl::vector& Args, bool ExecFDInterp, int ProgramFDFromEnv); /** * @brief Loads the FEX and application configurations for the application that is getting ready to run. diff --git a/Source/Tools/FEXLoader/FEXLoader.cpp b/Source/Tools/FEXLoader/FEXLoader.cpp index ea5188c85c..16f3add9b0 100644 --- a/Source/Tools/FEXLoader/FEXLoader.cpp +++ b/Source/Tools/FEXLoader/FEXLoader.cpp @@ -135,11 +135,17 @@ class AOTIRWriterFD final : public FEXCore::Context::AOTIRWriter { }; } // namespace AOTIR -void InterpreterHandler(fextl::string* Filename, const fextl::string& RootFS, fextl::vector* 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* 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 Header; @@ -151,47 +157,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 ShebangArguments {}; + std::string_view InterpreterLine {Data.begin() + 2, // strip off "#!" prefix + std::find(Data.begin(), Data.end(), '\n')}; + fextl::vector 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() { @@ -405,10 +404,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())) { + 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);