polkit: clean ups based on mvo's review.

canonical · Aug 23, 2017 · f3b9623 · f3b9623
1 parent 3e65cc5
commit f3b9623
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 11 deletions.
diff --git a/polkit/authority.go b/polkit/authority.go
@@ -67,11 +67,11 @@ func CheckAuthorizationForPid(pid uint32, actionId string, details map[string]st
 		Details: make(map[string]dbus.Variant),
 	}
 	subject.Details["pid"] = dbus.MakeVariant(pid)
-	if startTime, err := getStartTimeForPid(pid); err == nil {
-		subject.Details["start-time"] = dbus.MakeVariant(startTime)
-	} else {
+	startTime, err := getStartTimeForPid(pid)
+	if err != nil {
 		return false, err
 	}
+	subject.Details["start-time"] = dbus.MakeVariant(startTime)
 	return checkAuthorization(subject, actionId, details, flags)
 }
 

diff --git a/polkit/pid_start_time.go b/polkit/pid_start_time.go
@@ -23,22 +23,22 @@ package polkit
 import (
 	"fmt"
 	"io/ioutil"
-	"os"
 	"strconv"
 	"strings"
 )
 
 // getStartTimeForPid determines the start time for a given process ID
+func getStartTimeForPid(pid uint32) (uint64, error) {
+	filename := fmt.Sprintf("/proc/%d/stat", pid)
+	return getStartTimeForProcStatFile(filename)
+}
+
+// getStartTimeForProcStatFile determines the start time from a process stat file
 //
 // The implementation is intended to be compatible with polkit:
 //    https://cgit.freedesktop.org/polkit/tree/src/polkit/polkitunixprocess.c
-func getStartTimeForPid(pid uint32) (uint64, error) {
-	filename := fmt.Sprintf("/proc/%d/stat", pid)
-	file, err := os.Open(filename)
-	if err != nil {
-		return 0, err
-	}
-	data, err := ioutil.ReadAll(file)
+func getStartTimeForProcStatFile(filename string) (uint64, error) {
+	data, err := ioutil.ReadFile(filename)
 	if err != nil {
 		return 0, err
 	}
@@ -48,6 +48,10 @@ func getStartTimeForPid(pid uint32) (uint64, error) {
 	// name)' entry - since only this field can contain the ')'
 	// character, search backwards for this to avoid malicious
 	// processes trying to fool us
+	//
+	// See proc(5) man page for a description of the
+	// /proc/[pid]/stat file format and the meaning of the
+	// starttime field.
 	idx := strings.IndexByte(contents, ')')
 	if idx < 0 {
 		return 0, fmt.Errorf("cannot parse %s", filename)

diff --git a/polkit/pid_start_time_test.go b/polkit/pid_start_time_test.go
@@ -21,7 +21,9 @@
 package polkit
 
 import (
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"syscall"
 	"testing"
 
@@ -56,3 +58,14 @@ func (s *polkitSuite) TestGetStartTimeBadPid(c *check.C) {
 	c.Assert(err, check.ErrorMatches, "open .*: no such file or directory")
 	c.Check(startTime, check.Equals, uint64(0))
 }
+
+func (s *polkitSuite) TestProcStatParsing(c *check.C) {
+	filename := filepath.Join(c.MkDir(), "stat")
+	contents := []byte("18433 (cat) R 9732 18433 9732 34818 18433 4194304 96 0 1 0 0 0 0 0 20 0 1 0 123104764 7602176 182 18446744073709551615 94902526107648 94902526138492 140734457666896 0 0 0 0 0 0 0 0 0 17 5 0 0 0 0 0 94902528236168 94902528237760 94902542680064 140734457672267 140734457672287 140734457672287 140734457675759 0")
+	err := ioutil.WriteFile(filename, contents, 0644)
+	c.Assert(err, check.IsNil)
+
+	startTime, err := getStartTimeForProcStatFile(filename)
+	c.Assert(err, check.IsNil)
+	c.Check(startTime, check.Equals, uint64(123104764))
+}