Fix issue in GetProcessStartTime

[yongtang]

This fix tries to address the issue raised in docker:
moby/moby#27540

The issue was that GetProcessStartTime use space " "
to split the /proc/[pid]/stat and take the 22th value.

However, the 2th value is inside ( and ), and could
contain space. The following are two examples:

ubuntu@ubuntu:~/runc$ cat /proc/90286/stat
90286 (bash) S 90271 90286 90286 34818 90286 4194560 1412 1130576 4 0 2 1 2334 438 20 0 1 0 3093098 20733952 823 18446744073709551615 1 1 0 0 0 0 0 3670020 1266777851 0 0 0 17 1 0 0 0 0 0 0 0 0 0 0 0 0 0
ubuntu@ubuntu:~/runc$ cat /proc/89653/stat
89653 (gunicorn: maste) S 89630 89653 89653 0 -1 4194560 29689 28896 0 3 146 32 76 19 20 0 1 0 2971844 52965376 3920 18446744073709551615 1 1 0 0 0 0 0 16781312 137447943 0 0 0 17 1 0 0 0 0 0 0 0 0 0 0 0 0 0

This fix fixes this issue by removing the prefix before ),
then finding the 20th value (instead of 22th value).

Before the fix:

ubuntu@ubuntu:~/runc$ sudo ./runc state bb1be27a2fe3b8405639a2473b19677a988ef0e180c4e977969ade085b9d7a79
{
  "ociVersion": "1.0.0-rc2-dev",
  "id": "bb1be27a2fe3b8405639a2473b19677a988ef0e180c4e977969ade085b9d7a79",
  "pid": 0,
  "status": "stopped",
  "bundle": "/run/docker/libcontainerd/bb1be27a2fe3b8405639a2473b19677a988ef0e180c4e977969ade085b9d7a79",
  "rootfs": "/var/lib/docker/overlay2/8eadd0ca9cd66762e75cbe2b012f9521eb9a6a060a4d6ad444fc6fb8731edaa6/merged",
  "created": "2016-10-20T13:19:59.295011911Z"
}

After the fix:

ubuntu@ubuntu:~/runc$ sudo ./runc state bb1be27a2fe3b8405639a2473b19677a988ef0e180c4e977969ade085b9d7a79
{
  "ociVersion": "1.0.0-rc2-dev",
  "id": "bb1be27a2fe3b8405639a2473b19677a988ef0e180c4e977969ade085b9d7a79",
  "pid": 89653,
  "status": "running",
  "bundle": "/run/docker/libcontainerd/bb1be27a2fe3b8405639a2473b19677a988ef0e180c4e977969ade085b9d7a79",
  "rootfs": "/var/lib/docker/overlay2/8eadd0ca9cd66762e75cbe2b012f9521eb9a6a060a4d6ad444fc6fb8731edaa6/merged",
  "created": "2016-10-20T13:19:59.295011911Z"
}

Signed-off-by: Yong Tang yong.tang.github@outlook.com

[crosbymichael]

@yongtang thanks for doing this. Do you think you could make a quick test for this? If not, I can do it.

[crosbymichael]

here is the test I made that you can add to your PR:

diff --git a/libcontainer/system/proc.go b/libcontainer/system/proc.go
index a23f2c8..a0e9637 100644
--- a/libcontainer/system/proc.go
+++ b/libcontainer/system/proc.go
@@ -14,7 +14,10 @@ func GetProcessStartTime(pid int) (string, error) {
    if err != nil {
        return "", err
    }
+   return parseStartTime(string(data))
+}

+func parseStartTime(stat string) (string, error) {
    // the starttime is located at pos 22
    // from the man page
    //
@@ -34,7 +37,7 @@ func GetProcessStartTime(pid int) (string, error) {
    // 89653 (gunicorn: maste) S 89630 89653 89653 0 -1 4194560 29689 28896 0 3 146 32 76 19 20 0 1 0 2971844 52965376 3920 18446744073709551615 1 1 0 0 0 0 0 16781312 137447943 0 0 0 17 1 0 0 0 0 0 0 0 0 0 0 0 0 0

    // get parts after last `)`:
-   s := strings.Split(string(data), ")")
+   s := strings.Split(stat, ")")
    parts := strings.Split(strings.TrimSpace(s[len(s)-1]), " ")
    return parts[22-3], nil // starts at 3 (after the filename pos `2`)
 }
diff --git a/libcontainer/system/proc_test.go b/libcontainer/system/proc_test.go
new file mode 100644
index 0000000..ba91029
--- /dev/null
+++ b/libcontainer/system/proc_test.go
@@ -0,0 +1,20 @@
+package system
+
+import "testing"
+
+func TestParseStartTime(t *testing.T) {
+   data := map[string]string{
+       "4902 (gunicorn: maste) S 4885 4902 4902 0 -1 4194560 29683 29929 61 83 78 16 96 17 20 0 1 0 9126532 52965376 1903 18446744073709551615 4194304 7461796 140733928751520 140733928698072 139816984959091 0 0 16781312 137447943 1 0 0 17 3 0 0 9 0 0 9559488 10071156 33050624 140733928758775 140733928758945 140733928758945 140733928759264 0": "9126532",
+       "9534 (cat) R 9323 9534 9323 34828 9534 4194304 95 0 0 0 0 0 0 0 20 0 1 0 9214966 7626752 168 18446744073709551615 4194304 4240332 140732237651568 140732237650920 140570710391216 0 0 0 0 0 0 0 17 1 0 0 0 0 0 6340112 6341364 21553152 140732237653865 140732237653885 140732237653885 140732237656047 0":                                      "9214966",
+       "24767 (irq/44-mei_me) S 2 0 0 0 -1 2129984 0 0 0 0 0 0 0 0 -51 0 1 0 8722075 0 0 18446744073709551615 0 0 0 0 0 0 0 2147483647 0 0 0 0 17 1 50 1 0 0 0 0 0 0 0 0 0 0 0":                                                                                                                                                                         "8722075",
+   }
+   for line, startTime := range data {
+       st, err := parseStartTime(line)
+       if err != nil {
+           t.Fatal(err)
+       }
+       if startTime != st {
+           t.Fatalf("expected start time %q but received %q", startTime, st)
+       }
+   }
+}

[yongtang]

@crosbymichael Thanks a lot! I will update the PR shortly.

[yongtang]

Thanks @crosbymichael. The PR has been updated.

[crosbymichael]

LGTM

[mrunalp]

LGTM

[cyphar]

+1 Thanks, I was worried that this case would break things but didn't have a chance to test it. :D