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

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

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
>