From: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH v2 3/3] kernel-shark: When running as Root save all config settings in /root/
Date: Thu, 28 Nov 2019 13:29:02 +0200 [thread overview]
Message-ID: <7e2a221d-d1f0-ec91-ae42-6a9d48440fc5@gmail.com> (raw)
In-Reply-To: <20191127151310.680478f1@gandalf.local.home>
On 27.11.19 г. 22:13 ч., Steven Rostedt wrote:
> On Wed, 23 Oct 2019 15:21:45 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
>
>> If KernelShark is running with Root privileges, do not save the settings
>> in the standard location. Otherwise the configuration files will be owned
>> by Root and later the normal user will have no access to those files.
>>
>> The patch seems to do the right thing in all cases that I tested, however
>> there is definitely something that I do not understand. QDir::homePath()
>> always returns the path to the home of the normal user, even if I build
>> and run kernelshark as root (sudo -s).
>>
>> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>> ---
>> kernel-shark/src/KsMainWindow.cpp | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
>> index 3402764..bd6c338 100644
>> --- a/kernel-shark/src/KsMainWindow.cpp
>> +++ b/kernel-shark/src/KsMainWindow.cpp
>> @@ -69,7 +69,7 @@ KsMainWindow::KsMainWindow(QWidget *parent)
>> _contentsAction("Contents", this),
>> _bugReportAction("Report a bug", this),
>> _deselectShortcut(this),
>> - _settings("kernelshark.org", "Kernel Shark") // organization , application
>> + _settings(_getCacheDir() + "/setting.ini", QSettings::IniFormat)
>> {
>> setWindowTitle("Kernel Shark");
>> _createActions();
>> @@ -431,6 +431,9 @@ QString KsMainWindow::_getCacheDir()
>> dir = QStandardPaths::writableLocation(appCachePath);
>> dir += "/kernelshark";
>>
>> + if (geteuid() == 0)
>> + dir.replace(QDir::homePath(), "/root");
>> +
>> if (!QDir(dir).exists())
>> lamMakePath(false);
>> }
>
> I'll pull this patch in, but this assumes that root is always at /root.
> I've had machines where that was not the case. I wonder if we should
> add something like this on top of this patch. Not this change directly,
> (because this is me just writing C with at C++ compiler ;-), but
> something that is more the Qt way...
>
> diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
> index bd6c338f..56cf9b9b 100644
> --- a/kernel-shark/src/KsMainWindow.cpp
> +++ b/kernel-shark/src/KsMainWindow.cpp
> @@ -31,6 +31,43 @@
> #include "KsCaptureDialog.hpp"
> #include "KsAdvFilteringDialog.hpp"
>
> +static QString find_root_home(void)
> +{
> + FILE *fp = fopen("/etc/passwd", "r");
> + char *buf;
> + char *sav;
> + char *id;
> + size_t n;
> +
> + if (!fp)
> + return QString("/root");
> +
> + n = 0;
> + while (getline(&buf, &n, fp) != -1) {
> + /* user */
> + strtok_r(buf, ":", &sav);
> + /* type */
> + strtok_r(NULL, ":", &sav);
> + /* pid */
> + id = strtok_r(NULL, ":", &sav);
> + if (atoi(id) != 0) {
> + n = 0;
> + free(buf);
> + continue;
> + }
> + /* gid */
> + strtok_r(NULL, ":", &sav);
> + /* group */
> + strtok_r(NULL, ":", &sav);
> + /* home */
> + QString ret = QString(strtok_r(NULL, ":", &sav));
> + free(buf);
> + return ret;
> + }
> + free(buf);
> + return QString("/root");
> +}
> +
> /** Create KernelShark Main window. */
> KsMainWindow::KsMainWindow(QWidget *parent)
> : QMainWindow(parent),
> @@ -432,7 +469,8 @@ QString KsMainWindow::_getCacheDir()
> dir += "/kernelshark";
>
> if (geteuid() == 0)
> - dir.replace(QDir::homePath(), "/root");
> + dir.replace(QDir::homePath(),
> + find_root_home().toStdString().c_str());
>
> if (!QDir(dir).exists())
> lamMakePath(false);
>
> This reads the /etc/passwd file and searches for the pid of 0, and
> returns the home path for that user. On any error it just
> quietly defaults back to "/root".
>
Hi Steven,
Very good point. Thanks a lot!
I am sending a Qt-ish looking patch ;)
cheers,
Yordan
> -- Steve
>
prev parent reply other threads:[~2019-11-28 11:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-23 12:21 [PATCH v2 1/3] kernel-shark: Fix simple typo in the "File" menu Yordan Karadzhov (VMware)
2019-10-23 12:21 ` [PATCH v2 2/3] kernel-shark: Fix potential memory leak in libkshark-collection Yordan Karadzhov (VMware)
2019-11-27 18:54 ` Steven Rostedt
2019-11-28 9:25 ` Yordan Karadzhov (VMware)
2019-10-23 12:21 ` [PATCH v2 3/3] kernel-shark: When running as Root save all config settings in /root/ Yordan Karadzhov (VMware)
2019-11-27 20:13 ` Steven Rostedt
2019-11-28 11:29 ` Yordan Karadzhov (VMware) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7e2a221d-d1f0-ec91-ae42-6a9d48440fc5@gmail.com \
--to=y.karadz@gmail.com \
--cc=linux-trace-devel@vger.kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).