Re: [PATCH v2 3/3] kernel-shark: When running as Root save all config settings in /root/

["Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx>]

On 27.11.19 г. 22:13 ч., Steven Rostedt wrote:
> On Wed, 23 Oct 2019 15:21:45 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >   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