Skip to content

Commit c2d9668

Browse files
committed
libct/cg/OpenFile: fix openat2 vs top cgroup dir
Fix reading cgroup files from the top cgroup directory, i.e. /sys/fs/cgroup. The code was working for for any subdirectory of /sys/fs/cgroup, but for dir="/sys/fs/cgroup" a fallback (open and fstatfs) was used, because of the way the function worked with the dir argument. Fix those cases, and add unit tests to make sure they work. While at it, make the rules for dir and name components more relaxed, and add test cases for this, too. While at it, improve OpenFile documentation, and remove a duplicated doc comment for openFile. Without these fixes, the unit test fails the following cases: file_test.go:67: case {path:/sys/fs/cgroup name:cgroup.controllers}: fallback file_test.go:67: case {path:/sys/fs/cgroup/ name:cgroup.controllers}: openat2 /sys/fs/cgroup//cgroup.controllers: invalid cross-device link file_test.go:67: case {path:/sys/fs/cgroup/ name:/cgroup.controllers}: openat2 /sys/fs/cgroup///cgroup.controllers: invalid cross-device link file_test.go:67: case {path:/ name:/sys/fs/cgroup/cgroup.controllers}: fallback file_test.go:67: case {path:/ name:sys/fs/cgroup/cgroup.controllers}: fallback file_test.go:67: case {path:/sys/fs/cgroup/cgroup.controllers name:}: openat2 /sys/fs/cgroup/cgroup.controllers/: not a directory Here "fallback" means openat2-based implementation fails, and the fallback code is used (and works). Signed-off-by: Kir Kolyshkin <[email protected]>
1 parent 8772c4d commit c2d9668

File tree

2 files changed

+52
-14
lines changed

2 files changed

+52
-14
lines changed

libcontainer/cgroups/file.go

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"os"
8+
"path"
89
"strings"
910
"sync"
1011

@@ -13,7 +14,11 @@ import (
1314
)
1415

1516
// OpenFile opens a cgroup file in a given dir with given flags.
16-
// It is supposed to be used for cgroup files only.
17+
// It is supposed to be used for cgroup files only, and returns
18+
// an error if the file is not a cgroup file.
19+
//
20+
// Arguments dir and file are joined together to form an absolute path
21+
// to a file being opened.
1722
func OpenFile(dir, file string, flags int) (*os.File, error) {
1823
if dir == "" {
1924
return nil, fmt.Errorf("no directory specified for %s", file)
@@ -109,43 +114,43 @@ func prepareOpenat2() error {
109114
return prepErr
110115
}
111116

112-
// OpenFile opens a cgroup file in a given dir with given flags.
113-
// It is supposed to be used for cgroup files only.
114117
func openFile(dir, file string, flags int) (*os.File, error) {
115118
mode := os.FileMode(0)
116119
if TestMode && flags&os.O_WRONLY != 0 {
117120
// "emulate" cgroup fs for unit tests
118121
flags |= os.O_TRUNC | os.O_CREATE
119122
mode = 0o600
120123
}
124+
path := path.Join(dir, file)
121125
if prepareOpenat2() != nil {
122-
return openFallback(dir, file, flags, mode)
126+
return openFallback(path, flags, mode)
123127
}
124-
reldir := strings.TrimPrefix(dir, cgroupfsPrefix)
125-
if len(reldir) == len(dir) { // non-standard path, old system?
126-
return openFallback(dir, file, flags, mode)
128+
relPath := strings.TrimPrefix(path, cgroupfsPrefix)
129+
if len(relPath) == len(path) { // non-standard path, old system?
130+
return openFallback(path, flags, mode)
127131
}
128132

129-
relname := reldir + "/" + file
130-
fd, err := unix.Openat2(cgroupFd, relname,
133+
fd, err := unix.Openat2(cgroupFd, relPath,
131134
&unix.OpenHow{
132135
Resolve: resolveFlags,
133136
Flags: uint64(flags) | unix.O_CLOEXEC,
134137
Mode: uint64(mode),
135138
})
136139
if err != nil {
137-
return nil, &os.PathError{Op: "openat2", Path: dir + "/" + file, Err: err}
140+
return nil, &os.PathError{Op: "openat2", Path: path, Err: err}
138141
}
139142

140-
return os.NewFile(uintptr(fd), cgroupfsPrefix+relname), nil
143+
return os.NewFile(uintptr(fd), path), nil
141144
}
142145

143146
var errNotCgroupfs = errors.New("not a cgroup file")
144147

145-
// openFallback is used when openat2(2) is not available. It checks the opened
148+
// Can be changed by unit tests.
149+
var openFallback = openAndCheck
150+
151+
// openAndCheck is used when openat2(2) is not available. It checks the opened
146152
// file is on cgroupfs, returning an error otherwise.
147-
func openFallback(dir, file string, flags int, mode os.FileMode) (*os.File, error) {
148-
path := dir + "/" + file
153+
func openAndCheck(path string, flags int, mode os.FileMode) (*os.File, error) {
149154
fd, err := os.OpenFile(path, flags, mode)
150155
if err != nil {
151156
return nil, err

libcontainer/cgroups/file_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package cgroups
44

55
import (
6+
"errors"
67
"fmt"
78
"os"
89
"path/filepath"
@@ -40,3 +41,35 @@ func TestWriteCgroupFileHandlesInterrupt(t *testing.T) {
4041
}
4142
}
4243
}
44+
45+
func TestOpenat2(t *testing.T) {
46+
if !IsCgroup2UnifiedMode() {
47+
// The reason is many test cases below test opening files from
48+
// the top-level directory, where cgroup v1 has no files.
49+
t.Skip("test requires cgroup v2")
50+
}
51+
52+
// Make sure we test openat2, not its fallback.
53+
openFallback = func(_ string, _ int, _ os.FileMode) (*os.File, error) {
54+
return nil, errors.New("fallback")
55+
}
56+
defer func() { openFallback = openAndCheck }()
57+
58+
for _, tc := range []struct{ dir, file string }{
59+
{"/sys/fs/cgroup", "cgroup.controllers"},
60+
{"/sys/fs/cgroup", "/cgroup.controllers"},
61+
{"/sys/fs/cgroup/", "cgroup.controllers"},
62+
{"/sys/fs/cgroup/", "/cgroup.controllers"},
63+
{"/sys/fs/cgroup/user.slice", "cgroup.controllers"},
64+
{"/sys/fs/cgroup/user.slice/", "/cgroup.controllers"},
65+
{"/", "/sys/fs/cgroup/cgroup.controllers"},
66+
{"/", "sys/fs/cgroup/cgroup.controllers"},
67+
{"/sys/fs/cgroup/cgroup.controllers", ""},
68+
} {
69+
fd, err := OpenFile(tc.dir, tc.file, os.O_RDONLY)
70+
if err != nil {
71+
t.Errorf("case %+v: %v", tc, err)
72+
}
73+
fd.Close()
74+
}
75+
}

0 commit comments

Comments
 (0)