| Message ID | 20191023122145.14314-3-y.karadz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/3] kernel-shark: Fix simple typo in the "File" menu. | expand |
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". -- Steve
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 >
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); }
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(-)