From a4f3123c355962945f5a0b174f7b4f9804bf3fc3 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 6 Dec 2016 13:38:07 -0500 Subject: [PATCH] Fix thread safety of SelinuxEnabled and getSelinuxMountPoint Both suffered from different race conditions. SelinuxEnabled assigned selinuxEnabledChecked before selinuxEnabled. Thus racing callers could see the wrong selinuxEnabled. getSelinuxMountPoint assigned selinuxfs to "" before it know the right value. Thus racing could see "" improperly. The gate selinuxfs, enabled, and mclist all on the same lock --- libcontainer/selinux/selinux.go | 106 ++++++++++++++++++++++---------- 1 file changed, 73 insertions(+), 33 deletions(-) diff --git a/libcontainer/selinux/selinux.go b/libcontainer/selinux/selinux.go index fcaba1d29e6..ee86b791bf1 100644 --- a/libcontainer/selinux/selinux.go +++ b/libcontainer/selinux/selinux.go @@ -32,33 +32,73 @@ const ( stRdOnly = 0x01 ) +type selinuxState struct { + enabledSet bool + enabled bool + selinuxfsSet bool + selinuxfs string + mcsList map[string]bool + sync.Mutex +} + var ( - assignRegex = regexp.MustCompile(`^([^=]+)=(.*)$`) - mcsList = make(map[string]bool) - mcsLock sync.Mutex - selinuxfs = "unknown" - selinuxEnabled = false // Stores whether selinux is currently enabled - selinuxEnabledChecked = false // Stores whether selinux enablement has been checked or established yet + assignRegex = regexp.MustCompile(`^([^=]+)=(.*)$`) + state = selinuxState{ + mcsList: make(map[string]bool), + } ) type SELinuxContext map[string]string +func (s *selinuxState) setEnable(enabled bool) bool { + s.Lock() + defer s.Unlock() + s.enabledSet = true + s.enabled = enabled + return s.enabled +} + +func (s *selinuxState) getEnabled() bool { + s.Lock() + enabled := s.enabled + enabledSet := s.enabledSet + s.Unlock() + if enabledSet { + return enabled + } + + enabled = false + if fs := getSelinuxMountPoint(); fs != "" { + if con, _ := Getcon(); con != "kernel" { + enabled = true + } + } + return s.setEnable(enabled) +} + // SetDisabled disables selinux support for the package func SetDisabled() { - selinuxEnabled, selinuxEnabledChecked = false, true + state.setEnable(false) } -// getSelinuxMountPoint returns the path to the mountpoint of an selinuxfs -// filesystem or an empty string if no mountpoint is found. Selinuxfs is -// a proc-like pseudo-filesystem that exposes the selinux policy API to -// processes. The existence of an selinuxfs mount is used to determine -// whether selinux is currently enabled or not. -func getSelinuxMountPoint() string { - if selinuxfs != "unknown" { +func (s *selinuxState) setSELinuxfs(selinuxfs string) string { + s.Lock() + defer s.Unlock() + s.selinuxfsSet = true + s.selinuxfs = selinuxfs + return s.selinuxfs +} + +func (s *selinuxState) getSELinuxfs() string { + s.Lock() + selinuxfs := s.selinuxfs + selinuxfsSet := s.selinuxfsSet + s.Unlock() + if selinuxfsSet { return selinuxfs } - selinuxfs = "" + selinuxfs = "" f, err := os.Open("/proc/self/mountinfo") if err != nil { return selinuxfs @@ -91,21 +131,21 @@ func getSelinuxMountPoint() string { selinuxfs = "" } } - return selinuxfs + return s.setSELinuxfs(selinuxfs) +} + +// getSelinuxMountPoint returns the path to the mountpoint of an selinuxfs +// filesystem or an empty string if no mountpoint is found. Selinuxfs is +// a proc-like pseudo-filesystem that exposes the selinux policy API to +// processes. The existence of an selinuxfs mount is used to determine +// whether selinux is currently enabled or not. +func getSelinuxMountPoint() string { + return state.getSELinuxfs() } // SelinuxEnabled returns whether selinux is currently enabled. func SelinuxEnabled() bool { - if selinuxEnabledChecked { - return selinuxEnabled - } - selinuxEnabledChecked = true - if fs := getSelinuxMountPoint(); fs != "" { - if con, _ := Getcon(); con != "kernel" { - selinuxEnabled = true - } - } - return selinuxEnabled + return state.getEnabled() } func readConfig(target string) (value string) { @@ -283,19 +323,19 @@ func SelinuxGetEnforceMode() int { } func mcsAdd(mcs string) error { - mcsLock.Lock() - defer mcsLock.Unlock() - if mcsList[mcs] { + state.Lock() + defer state.Unlock() + if state.mcsList[mcs] { return fmt.Errorf("MCS Label already exists") } - mcsList[mcs] = true + state.mcsList[mcs] = true return nil } func mcsDelete(mcs string) { - mcsLock.Lock() - mcsList[mcs] = false - mcsLock.Unlock() + state.Lock() + defer state.Unlock() + state.mcsList[mcs] = false } func IntToMcs(id int, catRange uint32) string {