From 62cb001422a6a20cc47409e5c72fba85b8c0db15 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 7 Sep 2021 12:47:38 -0700 Subject: [PATCH 1/3] Lazy initialization for policyRoot Initializing policyRoot involves reading and parsing a config file, and it's not always used, so it's better to only init it when needed. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index e55445e..e4e1f6d 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -34,8 +34,6 @@ const ( xattrNameSelinux = "security.selinux" ) -var policyRoot = filepath.Join(selinuxDir, readConfig(selinuxTypeTag)) - type selinuxState struct { enabledSet bool enabled bool @@ -79,8 +77,20 @@ var ( // for attrPath() attrPathOnce sync.Once haveThreadSelf bool + + // for policyRoot() + policyRootOnce sync.Once + policyRootVal string ) +func policyRoot() string { + policyRootOnce.Do(func() { + policyRootVal = filepath.Join(selinuxDir, readConfig(selinuxTypeTag)) + }) + + return policyRootVal +} + func (s *selinuxState) setEnable(enabled bool) bool { s.Lock() defer s.Unlock() @@ -890,8 +900,7 @@ func openContextFile() (*os.File, error) { if f, err := os.Open(contextFile); err == nil { return f, nil } - lxcPath := filepath.Join(policyRoot, "/contexts/lxc_contexts") - return os.Open(lxcPath) + return os.Open(filepath.Join(policyRoot(), "/contexts/lxc_contexts")) } var labels, privContainerMountLabel = loadLabels() @@ -1179,15 +1188,14 @@ func getDefaultContextFromReaders(c *defaultSECtx) (string, error) { } func getDefaultContextWithLevel(user, level, scon string) (string, error) { - userPath := filepath.Join(policyRoot, selinuxUsersDir, user) - defaultPath := filepath.Join(policyRoot, defaultContexts) - + userPath := filepath.Join(policyRoot(), selinuxUsersDir, user) fu, err := os.Open(userPath) if err != nil { return "", err } defer fu.Close() + defaultPath := filepath.Join(policyRoot(), defaultContexts) fd, err := os.Open(defaultPath) if err != nil { return "", err From 8067137911be56dc2c5795d6f37b2f3a2fb2c2e8 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 7 Sep 2021 13:35:32 -0700 Subject: [PATCH 2/3] Lazy initialization for labels Perform lazy (on demand) init of labels, to reduce the impact of importing the package. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux.go | 4 ++++ go-selinux/selinux_linux.go | 41 +++++++++++++++++++------------- go-selinux/selinux_linux_test.go | 2 -- go-selinux/selinux_stub.go | 6 +++-- 4 files changed, 33 insertions(+), 20 deletions(-) diff --git a/go-selinux/selinux.go b/go-selinux/selinux.go index 9ffd77a..0eedcaa 100644 --- a/go-selinux/selinux.go +++ b/go-selinux/selinux.go @@ -38,6 +38,8 @@ var ( // CategoryRange allows the upper bound on the category range to be adjusted CategoryRange = DefaultCategoryRange + + privContainerMountLabel string ) // Context is a representation of the SELinux label broken into 4 parts @@ -280,5 +282,7 @@ func GetDefaultContextWithLevel(user, level, scon string) (string, error) { // PrivContainerMountLabel returns mount label for privileged containers func PrivContainerMountLabel() string { + // Make sure label is initialized. + _ = label("") return privContainerMountLabel } diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index e4e1f6d..1c172b1 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -81,6 +81,10 @@ var ( // for policyRoot() policyRootOnce sync.Once policyRootVal string + + // for label() + loadLabelsOnce sync.Once + labels map[string]string ) func policyRoot() string { @@ -903,13 +907,11 @@ func openContextFile() (*os.File, error) { return os.Open(filepath.Join(policyRoot(), "/contexts/lxc_contexts")) } -var labels, privContainerMountLabel = loadLabels() - -func loadLabels() (map[string]string, string) { - labels := make(map[string]string) +func loadLabels() { + labels = make(map[string]string) in, err := openContextFile() if err != nil { - return labels, "" + return } defer in.Close() @@ -933,30 +935,37 @@ func loadLabels() (map[string]string, string) { con, _ := NewContext(labels["file"]) con["level"] = fmt.Sprintf("s0:c%d,c%d", maxCategory-2, maxCategory-1) - reserveLabel(con.get()) - return labels, con.get() + privContainerMountLabel = con.get() + reserveLabel(privContainerMountLabel) +} + +func label(key string) string { + loadLabelsOnce.Do(func() { + loadLabels() + }) + return labels[key] } // kvmContainerLabels returns the default processLabel and mountLabel to be used // for kvm containers by the calling process. func kvmContainerLabels() (string, string) { - processLabel := labels["kvm_process"] + processLabel := label("kvm_process") if processLabel == "" { - processLabel = labels["process"] + processLabel = label("process") } - return addMcs(processLabel, labels["file"]) + return addMcs(processLabel, label("file")) } // initContainerLabels returns the default processLabel and file labels to be // used for containers running an init system like systemd by the calling process. func initContainerLabels() (string, string) { - processLabel := labels["init_process"] + processLabel := label("init_process") if processLabel == "" { - processLabel = labels["process"] + processLabel = label("process") } - return addMcs(processLabel, labels["file"]) + return addMcs(processLabel, label("file")) } // containerLabels returns an allocated processLabel and fileLabel to be used for @@ -966,9 +975,9 @@ func containerLabels() (processLabel string, fileLabel string) { return "", "" } - processLabel = labels["process"] - fileLabel = labels["file"] - readOnlyFileLabel = labels["ro_file"] + processLabel = label("process") + fileLabel = label("file") + readOnlyFileLabel = label("ro_file") if processLabel == "" || fileLabel == "" { return "", fileLabel diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index d7c1fd3..ab32dbd 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -42,8 +42,6 @@ func TestKVMLabels(t *testing.T) { t.Skip("SELinux not enabled, skipping.") } - t.Log(labels) - plabel, flabel := KVMContainerLabels() if plabel == "" { t.Log("Failed to read kvm label") diff --git a/go-selinux/selinux_stub.go b/go-selinux/selinux_stub.go index b7218a0..4265775 100644 --- a/go-selinux/selinux_stub.go +++ b/go-selinux/selinux_stub.go @@ -2,8 +2,6 @@ package selinux -const privContainerMountLabel = "" - func setDisabled() { } @@ -152,3 +150,7 @@ func disableSecOpt() []string { func getDefaultContextWithLevel(user, level, scon string) (string, error) { return "", nil } + +func label(_ string) string { + return "" +} From 335d2e29d1a055b51fb267e499a8ff823b4cfc09 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 7 Sep 2021 13:47:55 -0700 Subject: [PATCH 3/3] readConfig,loadLabels: stop using regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using a globally initialized regex incurs some overhead during package initialization, which is noticeable for all other packages importing it. Besides, there's no need to use regex to perform key=value split. Rewrite both functions without using regex, and add benchmarks, which show that the new code is faster: name old time/op new time/op delta ReadConfig-4 4.86µs ± 1% 3.46µs ± 2% -28.82% (p=0.002 n=6+6) LoadLabels-4 12.7µs ± 3% 6.8µs ± 1% -46.24% (p=0.002 n=6+6) name old alloc/op new alloc/op delta ReadConfig-4 5.08kB ± 0% 4.35kB ± 0% -14.33% (p=0.002 n=6+6) LoadLabels-4 6.49kB ± 0% 6.10kB ± 0% -6.07% (p=0.000 n=6+5) name old allocs/op new allocs/op delta ReadConfig-4 20.0 ± 0% 8.0 ± 0% -60.00% (p=0.002 n=6+6) LoadLabels-4 45.0 ± 0% 45.0 ± 0% ~ (all equal) Besides, the init overhead is eliminated: Before: init github.com/opencontainers/selinux/go-selinux @2.4 ms, 0.088 ms clock, 5016 bytes, 72 allocs After: init github.com/opencontainers/selinux/go-selinux @2.9 ms, 0.001 ms clock, 160 bytes, 8 allocs Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 25 +++++++++++++------------ go-selinux/selinux_linux_test.go | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 1c172b1..295b2bc 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -12,7 +12,6 @@ import ( "os" "path" "path/filepath" - "regexp" "strconv" "strings" "sync" @@ -68,7 +67,6 @@ const ( ) var ( - assignRegex = regexp.MustCompile(`^([^=]+)=(.*)$`) readOnlyFileLabel string state = selinuxState{ mcsList: make(map[string]bool), @@ -236,7 +234,7 @@ func readConfig(target string) string { scanner := bufio.NewScanner(in) for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) + line := bytes.TrimSpace(scanner.Bytes()) if len(line) == 0 { // Skip blank lines continue @@ -245,11 +243,12 @@ func readConfig(target string) string { // Skip comments continue } - if groups := assignRegex.FindStringSubmatch(line); groups != nil { - key, val := strings.TrimSpace(groups[1]), strings.TrimSpace(groups[2]) - if key == target { - return strings.Trim(val, "\"") - } + fields := bytes.SplitN(line, []byte{'='}, 2) + if len(fields) != 2 { + continue + } + if bytes.Equal(fields[0], []byte(target)) { + return string(bytes.Trim(fields[1], `"`)) } } return "" @@ -918,7 +917,7 @@ func loadLabels() { scanner := bufio.NewScanner(in) for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) + line := bytes.TrimSpace(scanner.Bytes()) if len(line) == 0 { // Skip blank lines continue @@ -927,10 +926,12 @@ func loadLabels() { // Skip comments continue } - if groups := assignRegex.FindStringSubmatch(line); groups != nil { - key, val := strings.TrimSpace(groups[1]), strings.TrimSpace(groups[2]) - labels[key] = strings.Trim(val, "\"") + fields := bytes.SplitN(line, []byte{'='}, 2) + if len(fields) != 2 { + continue } + key, val := bytes.TrimSpace(fields[0]), bytes.TrimSpace(fields[1]) + labels[string(key)] = string(bytes.Trim(val, `"`)) } con, _ := NewContext(labels["file"]) diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index ab32dbd..7dc1fe6 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -533,3 +533,17 @@ func BenchmarkCurrentLabel(b *testing.B) { } b.Log(l) } + +func BenchmarkReadConfig(b *testing.B) { + str := "" + for n := 0; n < b.N; n++ { + str = readConfig(selinuxTypeTag) + } + b.Log(str) +} + +func BenchmarkLoadLabels(b *testing.B) { + for n := 0; n < b.N; n++ { + loadLabels() + } +}