-
Notifications
You must be signed in to change notification settings - Fork 284
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
Chgans/allow preload injector override #543
base: master
Are you sure you want to change the base?
Conversation
This allow to overwrite the LD_PRELOAD, which can be useful when ld.so is in secure-execution mode. See https://mail.kdab.com/pipermail/gammaray-interest/2019-March/000317.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You reuse --injector-override
which is used by the gdb and lldb injectors to override the gdb/lldb executable. I don't know a lot about injection in general, but I wonder if it would make sense for the gdb/lldb injectors too, to allow overriding the probe-dll path. In that case it wouldn't be a good idea to reuse that commandline option. I think it's better to add a new one --probe-dll-override
. @vkrause what do you think?
@@ -68,13 +74,13 @@ bool PreloadInjector::launch(const QStringList &programAndArgs, const QString &p | |||
// ASAN requires to be loaded first, so check if the target uses that | |||
// and if so inject it before GammaRay | |||
QStringList ldPreload; | |||
foreach (const auto &lib, LibraryUtil::dependencies(exePath)) { | |||
for (const auto &lib: LibraryUtil::dependencies(exePath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do
const QVector<QByteArray> libraryDependencies = LibraryUtil::dependencies(exePath);
for (const auto &lib: libraryDependencies) {
else using range-for will detach the vector.
@@ -51,13 +55,15 @@ bool PreloadInjector::launch(const QStringList &programAndArgs, const QString &p | |||
{ | |||
Q_UNUSED(probeFunc); | |||
|
|||
const QString actualProbeDll = m_probeDllOverride.isEmpty() ? probeDll : m_probeDllOverride; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be better preserved at the caller side of this function, i.e. in Launcher::start. That requires the use of a separated commandline option though.
I think you are right, separating the command line arguments affecting the probe from those affecting the injector makes sense. The approach in general is fine IMHO, as discussed by email already. |
Oops, forgot about this PR.... |
I have signed NDA ages ago, and I have already contributed to GammaRay |
When i follow the link, i get:
|
yes, I'm sorry. I'm trying to use cla-assistant webhook to make things easier than "paper". still learning how to use it and I need to pre-approved those who already signed. |
This the PR for https://mail.kdab.com/pipermail/gammaray-interest/2019-March/000317.html.
It is untested so far, and i won't be able to test it before April. Nonetheless, you might want to comment on that.