Skip to content
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

New elevated privilege system #484

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
endif()

add_subdirectory(3rdparty)
add_subdirectory(scripts)

include_directories(${CMAKE_CURRENT_BINARY_DIR})
add_subdirectory(src)
Expand Down
7 changes: 1 addition & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -407,12 +407,7 @@ Consider tweaking /proc/sys/kernel/perf_event_paranoid:
2 - Disallow kernel profiling for unpriv
```

To workaround this limitation, hotspot can temporarily elevate the perf privileges.
This is achieved by applying
[these steps](https://superuser.com/questions/980632/run-perf-without-root-right),
bundled into [a script](scripts/elevate_perf_privileges.sh) that is run via `pkexec`, `kdesudo` or `kdesu`.
The resulting elevated privileges are also required for kernel tracing in general and Off-CPU profiling in
particular.
To workaround this limitation, hotspot can run perf itself with elevated privileges.

### Export File Format

Expand Down
8 changes: 0 additions & 8 deletions scripts/CMakeLists.txt

This file was deleted.

65 changes: 0 additions & 65 deletions scripts/elevate_perf_privileges.sh

This file was deleted.

1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ set(HOTSPOT_SRCS
perfoutputwidgettext.cpp
perfoutputwidgetkonsole.cpp
costcontextmenu.cpp
elevateprivilegeshelper.cpp
# ui files:
mainwindow.ui
aboutdialog.ui
Expand Down
186 changes: 186 additions & 0 deletions src/elevateprivilegeshelper.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
#include "elevateprivilegeshelper.h"
lievenhey marked this conversation as resolved.
Show resolved Hide resolved

#include <QDebug>
#include <QFile>
#include <QSocketNotifier>
#include <QStandardPaths>
#include <QUuid>

#include <fcntl.h>
#include <signal.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <vector>

InitiallyStoppedProcess::~InitiallyStoppedProcess()
{
kill();
}

bool InitiallyStoppedProcess::reset(const QString& exePath, const QStringList& exeOptions,
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
const QString& workingDirectory)
{
kill();

// convert arguments and working dir into what the C API needs

std::vector<QByteArray> args;
args.reserve(exeOptions.size() + 1);
args.emplace_back(exePath.toLocal8Bit());
for (const auto& opt : exeOptions)
args.emplace_back(opt.toLocal8Bit());
const auto wd = workingDirectory.toLocal8Bit();

std::vector<char*> argsArray(args.size() + 1);
for (size_t i = 0; i < args.size(); ++i)
argsArray[i] = args[i].data();
argsArray.back() = nullptr;

// fork
m_pid = fork();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forking means we COW the whole address space which can be really slow when that is large. that would e.g. be the case when you start hotspot, analyze some large file and then rerun the record from there.

is there a way this could use QProcess such that we can leverage vfork/posix_spawn when available? or is there maybe something like that in boost which we could use to get this behavior in a more efficient manner?

see also: https://codereview.qt-project.org/c/qt/qtbase/+/417829

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure this could be improved. Don't know what's available in boost for that. vfork seems a bit dangerous to me, for posix_spawn I suppose one would need to move this initially-stopped-exec functionality in a separate helper binary or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can only use vfork if we create a new binary, since vfork only allows for __exit() and exec* calls. I think clone with CLONE_WM | CLONE_VFORK might be the easiest way to solve this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, VFORK suspends the calling process until exec is called, which we do not want here at all. I don't think VFORK is usable for this. posix_spawn and a helper process should work though.


if (m_pid == 0) { // inside child process
// change working dir
if (!wd.isEmpty() && chdir(wd.data()) != 0)
qFatal("Failed to change working directory to %s", wd.data());

// stop self
raise(SIGSTOP);

// exec
execvp(argsArray[0], argsArray.data());
qFatal("Failed to exec %s", argsArray[0]);
} else if (m_pid < 0) {
qCritical("Failed to fork (?)");
return false;
}

return true;
}

bool InitiallyStoppedProcess::run()
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
{
if (m_pid <= 0)
return false;

// wait for child to be stopped

int wstatus;
waitpid(m_pid, &wstatus, WUNTRACED);
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
if (!WIFSTOPPED(wstatus)) {
m_pid = -1;
return false;
}

// continue

::kill(m_pid, SIGCONT);
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

void InitiallyStoppedProcess::terminate()
{
if (m_pid > 0)
::kill(m_pid, SIGTERM);
}

void InitiallyStoppedProcess::kill()
{
if (m_pid > 0) {
::kill(m_pid, SIGKILL);
waitpid(m_pid, nullptr, 0);
m_pid = -1;
}
}

PerfControlFifoWrapper::~PerfControlFifoWrapper()
{
close();
}

bool PerfControlFifoWrapper::open()
{
close();

QString fifoParentPath = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation);
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
if (fifoParentPath.isEmpty())
fifoParentPath = QStandardPaths::writableLocation(QStandardPaths::TempLocation);
lievenhey marked this conversation as resolved.
Show resolved Hide resolved

const auto fifoBasePath =
QStringLiteral("%1/hotspot-%2-%3-perf")
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
.arg(fifoParentPath, QString::number(getpid()), QUuid::createUuid().toString().mid(1, 6));
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
m_ctlFifoPath = fifoBasePath + QStringLiteral("-control.fifo");
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
m_ackFifoPath = fifoBasePath + QStringLiteral("-ack.fifo");

if (mkfifo(m_ctlFifoPath.toLocal8Bit().data(), 0600) != 0) {
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
qCritical() << "Cannot create fifo" << m_ctlFifoPath;
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
if (mkfifo(m_ackFifoPath.toLocal8Bit().data(), 0600) != 0) {
qCritical() << "Cannot create fifo" << m_ackFifoPath;
return false;
}

m_ctlFifoFd = ::open(m_ctlFifoPath.toLocal8Bit().data(), O_RDWR);
if (m_ctlFifoFd < 0) {
qCritical() << "Cannot open fifo" << m_ctlFifoPath;
return false;
}
m_ackFifoFd = ::open(m_ackFifoPath.toLocal8Bit().data(), O_RDONLY | O_NONBLOCK);
if (m_ackFifoFd < 0) {
qCritical() << "Cannot open fifo" << m_ackFifoPath;
return false;
}

return true;
}

void PerfControlFifoWrapper::start()
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
{
if (m_ctlFifoFd < 0)
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
return;

m_ackReady = new QSocketNotifier(m_ackFifoFd, QSocketNotifier::Read);
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
connect(m_ackReady, &QSocketNotifier::activated, this, [this]() {
char buf[10];
read(m_ackFifoFd, buf, sizeof(buf));
emit started();
m_ackReady->disconnect(this);
});

const char start_cmd[] = "enable\n";
write(m_ctlFifoFd, start_cmd, sizeof(start_cmd) - 1);
}

void PerfControlFifoWrapper::stop()
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
{
if (m_ctlFifoFd < 0)
return;
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
const char stop_cmd[] = "stop\n";
write(m_ctlFifoFd, stop_cmd, sizeof(stop_cmd) - 1);
}

void PerfControlFifoWrapper::close()
{
if (m_ackReady) {
delete m_ackReady;
m_ackReady = nullptr;
}
if (m_ctlFifoFd >= 0) {
::close(m_ctlFifoFd);
m_ctlFifoFd = -1;
}
if (m_ackFifoFd >= 0) {
::close(m_ackFifoFd);
m_ackFifoFd = -1;
}
if (!m_ctlFifoPath.isEmpty()) {
QFile::remove(m_ctlFifoPath);
m_ctlFifoPath.clear();
}
if (!m_ackFifoPath.isEmpty()) {
QFile::remove(m_ackFifoPath);
m_ackFifoPath.clear();
}
}
76 changes: 76 additions & 0 deletions src/elevateprivilegeshelper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
SPDX-FileCopyrightText: Zeno Endemann <[email protected]>
SPDX-FileCopyrightText: 2016-2023 Klarälvdalens Datakonsult AB, a KDAB Group company, [email protected]
lievenhey marked this conversation as resolved.
Show resolved Hide resolved

SPDX-License-Identifier: GPL-2.0-or-later
*/

#pragma once

#include <QObject>
#include <QString>
#include <QStringList>

#include <unistd.h>

class QSocketNotifier;

class InitiallyStoppedProcess
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
{
public:
InitiallyStoppedProcess() = default;
~InitiallyStoppedProcess();

InitiallyStoppedProcess(const InitiallyStoppedProcess&) = delete;
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
InitiallyStoppedProcess operator=(const InitiallyStoppedProcess&) = delete;

pid_t processPID() const
{
return m_pid;
}

bool reset(const QString& exePath, const QStringList& exeOptions, const QString& workingDirectory);
bool run();
void terminate();
void kill();

private:
pid_t m_pid = -1;
};

class PerfControlFifoWrapper : public QObject
lievenhey marked this conversation as resolved.
Show resolved Hide resolved
{
Q_OBJECT

public:
using QObject::QObject;
~PerfControlFifoWrapper();

bool isOpen() const
{
return m_ctlFifoFd >= 0;
}
QString controlFifoPath() const
{
return m_ctlFifoPath;
}
QString ackFifoPath() const
{
return m_ackFifoPath;
}

bool open();
void start();
void stop();
void close();

signals:
void started();

private:
QSocketNotifier* m_ackReady = nullptr;
QString m_ctlFifoPath;
QString m_ackFifoPath;
int m_ctlFifoFd = -1;
int m_ackFifoFd = -1;
};
Loading