D11235: [DrKonqi] Request change of ptrace scope from KCrash

[Oswald Buddenhagen]

ossi requested changes to this revision.
ossi added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> debuggerlaunchers.cpp:57
>      if ( pid > 0 ) {
> +        queryPtrace(pid);
>          m_monitor->startMonitoring(pid);

according to my reading of the documentation (https://www.kernel.org/doc/Documentation/security/Yama.txt) and the kernel source, this is unnecessary in this branch, as descendants of the assigned ptracer have the right as well (that's why the "normal" backtrace creation works without your patch), and not even detaching breaks the chain.

> queryptrace.cpp:67
> +
> +            if (select(sfd + 1, &set, NULL, NULL, &timeout) > 0 &&
> +                    read(sfd, msg, 3) > 0 && strcmp(msg, "OK") == 0) {

use poll() here, too.

> queryptrace.cpp:68
> +            if (select(sfd + 1, &set, NULL, NULL, &timeout) > 0 &&
> +                    read(sfd, msg, 3) > 0 && strcmp(msg, "OK") == 0) {
> +                qCInfo(DRKONQI_LOG) << "ptrace granted by debugged process";

the current version of the complementary patch just echoes back the pid, so this cannot possibly work.

> croick wrote in queryptrace.cpp:26
> QTemporaryDir will not work, since the location is set in KCrash.

the location is still bogus, though. see comment on the other diff.

> queryptrace.h:22
> +
> +/** In Linux try to make the process attach to the debugger */
> +void queryPtrace(qint64 pid);

no. "on linux, tell the process to allow the debugger to attach to it".

> queryptrace.h:23
> +/** In Linux try to make the process attach to the debugger */
> +void queryPtrace(qint64 pid);
> +

that's a rather misleading function name. setPtracer() would reflect the purpose and upstream naming.

> croick wrote in crashtest.cpp:138
> That reasoning does not seem to be true. The output can still be read when started "normally".

only if kdeinit was started from the same console, which you cannot assume.

also, this change wouldn't belong into this patch anyway.

REPOSITORY
  R871 DrKonqi

REVISION DETAIL
  https://phabricator.kde.org/D11235

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, sukalyanbanga, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190114/8ee3793b/attachment-0001.html>