mib created this revision.
mib added reviewers: JDevlieghere, labath.
mib added a project: LLDB.
Herald added subscribers: lldb-commits, emaste.
mib marked an inline comment as done.
mib added inline comments.
mib edited the summary of this revision.
================
Comment at: lldb/test/API/commands/target/basic/TestTargetCommand.py:343
+ def test_target_create_unowned_core_file(self):
+ self.expect("target create -c ~root", error=True,
+ substrs=["core file '", "' is not readable"])
----------------
Currently, I try to open the root user home directory, since I sure it's not
readable by other users on UNIX systems.
I went this route, because doing a `os.chown` requires privileges, and the test
suites doesn't run in a privileged mode.
I'm open to suggestions on how I could test this on different platforms.
When trying to read a core file that is not owned by the user running lldb
and that doesn't have read permission on the file, we can a misleading
error message
Unable to find process plug-in for core file
This is due to the fact that currently, lldb doesn't check the file
ownership. And when trying to to open and read a core file, the syscall
fails, which prevents a process to be created.
This patch introduces the `FileSystem::ReadableByCurrentUser` method
that checks if the file has readable permissions and its ownership.
This patch also changes the error messages to be more accurate.
rdar://42630030
Signed-off-by: Med Ismail Bennani <medismail.benn...@gmail.com>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78712
Files:
lldb/include/lldb/Host/FileSystem.h
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/test/API/commands/target/basic/TestTargetCommand.py
Index: lldb/test/API/commands/target/basic/TestTargetCommand.py
===================================================================
--- lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -337,6 +337,12 @@
self.expect("target create -c '" + tf.name + "'", error=True,
substrs=["core file '", "' is not readable"])
+ @skipIfWindows
+ @no_debug_info_test
+ def test_target_create_unowned_core_file(self):
+ self.expect("target create -c ~root", error=True,
+ substrs=["core file '", "' is not readable"])
+
@no_debug_info_test
def test_target_create_nonexistent_sym_file(self):
self.expect("target create -s doesntexist doesntexisteither", error=True,
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===================================================================
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -91,20 +91,17 @@
// Resolve any executable within a bundle on MacOSX
Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
- if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+ if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+ error.SetErrorStringWithFormat(
+ "unable to find executable for '%s'",
+ resolved_module_spec.GetFileSpec().GetPath().c_str());
+ else if (!FileSystem::Instance().ReadableByCurrentUser(
+ resolved_module_spec.GetFileSpec())) {
+ error.SetErrorStringWithFormat(
+ "executable '%s' is not readable by the current user",
+ resolved_module_spec.GetFileSpec().GetPath().c_str());
+ } else
error.Clear();
- else {
- const uint32_t permissions = FileSystem::Instance().GetPermissions(
- resolved_module_spec.GetFileSpec());
- if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
- error.SetErrorStringWithFormat(
- "executable '%s' is not readable",
- resolved_module_spec.GetFileSpec().GetPath().c_str());
- else
- error.SetErrorStringWithFormat(
- "unable to find executable for '%s'",
- resolved_module_spec.GetFileSpec().GetPath().c_str());
- }
} else {
if (m_remote_platform_sp) {
error =
@@ -188,7 +185,7 @@
}
if (error.Fail() || !exe_module_sp) {
- if (FileSystem::Instance().Readable(
+ if (FileSystem::Instance().ReadableByCurrentUser(
resolved_module_spec.GetFileSpec())) {
error.SetErrorStringWithFormat(
"'%s' doesn't contain any '%s' platform architectures: %s",
@@ -196,7 +193,7 @@
GetPluginName().GetCString(), arch_names.GetData());
} else {
error.SetErrorStringWithFormat(
- "'%s' is not readable",
+ "'%s' is not readable by the current user",
resolved_module_spec.GetFileSpec().GetPath().c_str());
}
}
Index: lldb/source/Host/common/FileSystem.cpp
===================================================================
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/File.h"
+#include "lldb/Host/HostInfo.h"
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/TildeExpressionResolver.h"
@@ -491,3 +493,13 @@
m_collector->addFile(file);
}
}
+
+bool FileSystem::ReadableByCurrentUser(const FileSpec &file_spec) const {
+ return Readable(file_spec) &&
+ Instance().Open(file_spec, File::eOpenOptionRead);
+}
+
+bool FileSystem::ReadableByCurrentUser(const llvm::Twine &file_path) const {
+ FileSpec file(file_path.getSingleStringRef());
+ return Readable(file_path) && Instance().Open(file, File::eOpenOptionRead);
+}
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -277,9 +277,10 @@
result.SetStatus(eReturnStatusFailed);
return false;
}
- if (!FileSystem::Instance().Readable(core_file)) {
- result.AppendErrorWithFormat("core file '%s' is not readable",
- core_file.GetPath().c_str());
+ if (!FileSystem::Instance().ReadableByCurrentUser(core_file)) {
+ result.AppendErrorWithFormat(
+ "core file '%s' is not readable by the current user.",
+ core_file.GetPath().c_str());
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -289,9 +290,10 @@
FileSpec symfile(m_symbol_file.GetOptionValue().GetCurrentValue());
if (symfile) {
if (FileSystem::Instance().Exists(symfile)) {
- if (!FileSystem::Instance().Readable(symfile)) {
- result.AppendErrorWithFormat("symbol file '%s' is not readable",
- symfile.GetPath().c_str());
+ if (!FileSystem::Instance().ReadableByCurrentUser(symfile)) {
+ result.AppendErrorWithFormat(
+ "symbol file '%s' is not readable by the current user.",
+ symfile.GetPath().c_str());
result.SetStatus(eReturnStatusFailed);
return false;
}
Index: lldb/include/lldb/Host/FileSystem.h
===================================================================
--- lldb/include/lldb/Host/FileSystem.h
+++ lldb/include/lldb/Host/FileSystem.h
@@ -189,6 +189,9 @@
void Collect(const FileSpec &file_spec);
void Collect(const llvm::Twine &file);
+ bool ReadableByCurrentUser(const FileSpec &file_spec) const;
+ bool ReadableByCurrentUser(const llvm::Twine &file) const;
+
private:
static llvm::Optional<FileSystem> &InstanceImpl();
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
