Merge pull request #774 from safing/fix/layered-profile-deadlock

Fix layered profile deadlocks

firewall/master.go

@@ -270,10 +270,10 @@ func checkEndpointListsForSystemResolverDNSRequests(ctx context.Context, conn *n
 	var profileEndpoints endpoints.Endpoints
 	var optionKey string
 	if conn.Inbound {
-		profileEndpoints = p.LocalProfile().GetServiceEndpoints()
+		profileEndpoints = p.LocalProfileWithoutLocking().GetServiceEndpoints()
 		optionKey = profile.CfgOptionServiceEndpointsKey
 	} else {
-		profileEndpoints = p.LocalProfile().GetEndpoints()
+		profileEndpoints = p.LocalProfileWithoutLocking().GetEndpoints()
 		optionKey = profile.CfgOptionEndpointsKey
 	}
 

nameserver/nameserver.go

@@ -231,7 +231,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg)
 	// the resolving as it wishes.
 	if responder, ok := conn.Reason.Context.(nsutil.Responder); ok {
 		tracer.Infof("nameserver: handing over request for %s to special filter responder: %s", q.ID(), conn.Reason.Msg)
-		return reply(responder)
+		return reply(responder, conn)
 	}
 
 	// Check if there is a Verdict to act upon.

process/find.go

@@ -81,7 +81,7 @@ func GetNetworkHost(ctx context.Context, remoteIP net.IP) (process *Process, err
 	}
 
 	// Assign profile to process.
-	networkHost.LocalProfileKey = networkHostProfile.Key()
+	networkHost.PrimaryProfileID = networkHostProfile.ScopedID()
 	networkHost.profile = networkHostProfile.LayeredProfile()
 
 	if networkHostProfile.Name == "" {

process/process.go

@@ -28,7 +28,8 @@ type Process struct {
 	record.Base
 	sync.Mutex
 
-	// Constant attributes.
+	// Process attributes.
+	// Don't change; safe for concurrent access.
 
 	Name      string
 	UserID    int
@@ -46,8 +47,13 @@ type Process struct {
 	// based on any of the previous attributes.
 	SpecialDetail string
 
-	LocalProfileKey string
+	// Profile attributes.
-	profile         *profile.LayeredProfile
+	// Once set, these don't change; safe for concurrent access.
+
+	// PrimaryProfileID holds the scoped ID of the primary profile.
+	PrimaryProfileID string
+	// profile holds the layered profile based on the primary profile.
+	profile *profile.LayeredProfile
 
 	// Mutable attributes.
 
@@ -93,6 +99,8 @@ func (p *Process) Equal(other *Process) bool {
 	return p.IsIdentified() && other.IsIdentified() && p.Pid == other.Pid
 }
 
+const systemResolverScopedID = string(profile.SourceLocal) + "/" + profile.SystemResolverProfileID
+
 // IsSystemResolver is a shortcut to check if the process is or belongs to the
 // system resolver and needs special handling.
 func (p *Process) IsSystemResolver() bool {
@@ -101,14 +109,8 @@ func (p *Process) IsSystemResolver() bool {
 		return false
 	}
 
-	// Check if local profile exists.
-	localProfile := p.profile.LocalProfile()
-	if localProfile == nil {
-		return false
-	}
-
 	// Check ID.
-	return localProfile.ID == profile.SystemResolverProfileID
+	return p.PrimaryProfileID == systemResolverScopedID
 }
 
 // GetLastSeen returns the unix timestamp when the process was last seen.

process/profile.go

@@ -89,7 +89,7 @@ func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) {
 	}
 
 	// Assign profile to process.
-	p.LocalProfileKey = localProfile.Key()
+	p.PrimaryProfileID = localProfile.ScopedID()
 	p.profile = localProfile.LayeredProfile()
 
 	return true, nil

profile/profile-layered.go

@@ -168,6 +168,17 @@ func (lp *LayeredProfile) LocalProfile() *Profile {
 	return lp.localProfile
 }
 
+// LocalProfileWithoutLocking returns the local profile associated with this
+// layered profile, but without locking the layered profile.
+// This method my only be used when the caller already has a lock on the layered profile.
+func (lp *LayeredProfile) LocalProfileWithoutLocking() *Profile {
+	if lp == nil {
+		return nil
+	}
+
+	return lp.localProfile
+}
+
 // increaseRevisionCounter increases the revision counter and pushes the
 // layered profile to listeners.
 func (lp *LayeredProfile) increaseRevisionCounter(lock bool) (revisionCounter uint64) { //nolint:unparam // This is documentation.