diff mbox

[RESEND] tools/power/x86/intel_pstate_tracer: Adjust directory permissions

Message ID 1492355834-7793-1-git-send-email-dsmythies@telus.net (mailing list archive)
State Deferred
Headers show

Commit Message

Doug Smythies April 16, 2017, 3:17 p.m. UTC
Depending on what is being done, the intel_pstate_tracer.py script
needs to be run as root, or can be run as a regular user.
If run the first time as root the results directory will be
incorrect for any subsequent run as a regular user. For any run
as root the specific testname subdirectory will not allow any
subsequent file saves by a regular user. Typically, and for example,
the regular user might be attempting to save a .csv file converted to
a spreadsheet with added calculations or graphs.

Override the default folder permissions.

Signed-off-by: Doug Smythies <dsmythies@telus.net>
---
 tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Rafael J. Wysocki April 16, 2017, 10:57 p.m. UTC | #1
On Sun, Apr 16, 2017 at 5:17 PM, Doug Smythies <doug.smythies@gmail.com> wrote:
> Depending on what is being done, the intel_pstate_tracer.py script
> needs to be run as root, or can be run as a regular user.
> If run the first time as root the results directory will be
> incorrect for any subsequent run as a regular user.

Which is OK.

For security reasons, non-root should not be able to modify root-owned
directories.

Thanks,
Rafael
Doug Smythies April 18, 2017, 12:11 a.m. UTC | #2
On 2107.04.16 15:57 Rafael J. Wysocki wrote:
> On Sun, Apr 16, 2017 at 5:17 PM, Doug Smythies <doug.smythies@gmail.com> wrote:
>> Depending on what is being done, the intel_pstate_tracer.py script
>> needs to be run as root, or can be run as a regular user.
>> If run the first time as root the results directory will be
>> incorrect for any subsequent run as a regular user.
>
> Which is OK.
>
> For security reasons, non-root should not be able to modify root-owned
> directories.

Hi Rafael,

I do not see a security issue here.

The objective was to merge what used to be
two steps (with the old, never released, post processing tools)
into one step. The only reason "root" was ever needed was for
the actual trace step. In the past everything else could be
done as a regular user. Even when the two step method is used
and we are processing a previously acquired (as "root"), it is
preferred to do so as a regular user.

Anyway, in a minute I will send a version 2 of the patch, where
the user and group IDs are changed to regular user, rather than
changing permissions.

... Doug
diff mbox

Patch

diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
index fd706ac..7c855508 100755
--- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
@@ -517,13 +517,19 @@  else:
         cpu_mask[i] = 1
 
 if not os.path.exists('results'):
+    # Setting permissions with the mode option doesn't work on all platforms
     os.mkdir('results')
+    # so use chmod to set them. Needed because user may or may not be root.
+    os.chmod('results', 0777)
 
 os.chdir('results')
 if os.path.exists(testname):
     print('The test name directory already exists. Please provide a unique test name. Test re-run not supported, yet.')
     sys.exit()
+# Setting permissions with the mode option doesn't work on all platforms
 os.mkdir(testname)
+os.chmod(testname, 0777)
+# so use chmod to set them. Needed because user may or may not be root.
 os.chdir(testname)
 
 # Temporary (or perhaps not)