-
Notifications
You must be signed in to change notification settings - Fork 13.3k
release/20.x: [lldb] Use correct path for lldb-server executable (#131519) #134072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
llvmbot
wants to merge
1
commit into
llvm:release/20.x
Choose a base branch
from
llvmbot:issue131519
base: release/20.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+11
−7
Conversation
This file contains 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
Hey, This solves an issue where running lldb-server-20 with a non-absolute path (for example, when it's installed into `/usr/bin` and the user runs it as `lldb-server-20 ...` and not `/usr/bin/lldb-server-20 ...`) fails with `error: spawn_process failed: execve failed: No such file or directory`. The underlying issue is that when run that way, it attempts to execute a binary named `lldb-server-20` from its current directory. This is also a mild security hazard because lldb-server is often being run as root in the directory /tmp, meaning that an unprivileged user can create the file /tmp/lldb-server-20 and lldb-server will execute it as root. (although, well, it's a debugging server we're talking about, so that may not be a real concern) I haven't previously contributed to this project; if you want me to change anything in the code please don't hesitate to let me know. (cherry picked from commit 945c494)
@labath What do you think about merging this PR to the release branch? |
@llvm/pr-subscribers-lldb Author: None (llvmbot) ChangesBackport 945c494 Requested by: @DavidSpickett Full diff: https://github.com/llvm/llvm-project/pull/134072.diff 1 Files Affected:
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp
index 880b45b989b9c..51174a0f443c3 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -31,6 +31,7 @@
#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostGetOpt.h"
+#include "lldb/Host/HostInfo.h"
#include "lldb/Host/MainLoop.h"
#include "lldb/Host/OptionParser.h"
#include "lldb/Host/Socket.h"
@@ -256,8 +257,9 @@ static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
printf("Disconnected.\n");
}
-static Status spawn_process(const char *progname, const Socket *conn_socket,
- uint16_t gdb_port, const lldb_private::Args &args,
+static Status spawn_process(const char *progname, const FileSpec &prog,
+ const Socket *conn_socket, uint16_t gdb_port,
+ const lldb_private::Args &args,
const std::string &log_file,
const StringRef log_channels, MainLoop &main_loop) {
Status error;
@@ -267,9 +269,10 @@ static Status spawn_process(const char *progname, const Socket *conn_socket,
ProcessLaunchInfo launch_info;
- FileSpec self_spec(progname, FileSpec::Style::native);
- launch_info.SetExecutableFile(self_spec, true);
+ launch_info.SetExecutableFile(prog, false);
+ launch_info.SetArg0(progname);
Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(progname);
self_args.AppendArgument(llvm::StringRef("platform"));
self_args.AppendArgument(llvm::StringRef("--child-platform-fd"));
self_args.AppendArgument(llvm::to_string(shared_socket.GetSendableFD()));
@@ -551,9 +554,10 @@ int main_platform(int argc, char *argv[]) {
log_channels, &main_loop,
&platform_handles](std::unique_ptr<Socket> sock_up) {
printf("Connection established.\n");
- Status error = spawn_process(progname, sock_up.get(),
- gdbserver_port, inferior_arguments,
- log_file, log_channels, main_loop);
+ Status error = spawn_process(
+ progname, HostInfo::GetProgramFileSpec(), sock_up.get(),
+ gdbserver_port, inferior_arguments, log_file, log_channels,
+ main_loop);
if (error.Fail()) {
Log *log = GetLog(LLDBLog::Platform);
LLDB_LOGF(log, "spawn_process failed: %s", error.AsCString());
|
I think it's fine, but I believe you'll also need #133093 for this to make a difference. |
Thank you, I wanted this to be backported but wasn't sure how to do it :) |
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.
Backport 945c494
Requested by: @DavidSpickett