maps: finalize scalar maps during creation, defer prog and outer maps

`cl.loadMap()` now finalizes maps that can be finalized right after creation,
aka. 'scalar' maps without references to other bpf resources. Prog maps or
outer maps are populated at a later stage.

After doing a first pass over all requested Maps and Programs, a separate pass
is run by `cl.populateMaps()` (renamed to `populateDeferredMaps()`) that only
considers prog maps and outer maps, lazy-loads dependencies and fully resolves
Spec.Contents, then finalizes the Map.

This fixes .rodata being frozen after the verifier has run, which not only
defeats the point of having constants, but also causes verifier errors if
a runtime-provided constant is used as a return code or helper argument that
must be proven to be within a certain range during verification.

Signed-off-by: Timo Beckers <timo@isovalent.com>

collection.go

@@ -309,7 +309,7 @@ func (cs *CollectionSpec) LoadAndAssign(to interface{}, opts *CollectionOptions)
 	}
 
 	// Populate the requested maps. Has a chance of lazy-loading other dependent maps.
-	if err := loader.populateMaps(); err != nil {
+	if err := loader.populateDeferredMaps(); err != nil {
 		return err
 	}
 
@@ -389,7 +389,7 @@ func NewCollectionWithOptions(spec *CollectionSpec, opts CollectionOptions) (*Co
 
 	// Maps can contain Program and Map stubs, so populate them after
 	// all Maps and Programs have been successfully loaded.
-	if err := loader.populateMaps(); err != nil {
+	if err := loader.populateDeferredMaps(); err != nil {
 		return nil, err
 	}
 
@@ -471,6 +471,15 @@ func (cl *collectionLoader) loadMap(mapName string) (*Map, error) {
 		return nil, fmt.Errorf("map %s: %w", mapName, err)
 	}
 
+	// Finalize 'scalar' maps that don't refer to any other eBPF resources
+	// potentially pending creation. This is needed for frozen maps like .rodata
+	// that need to be finalized before invoking the verifier.
+	if !mapSpec.Type.canStoreMapOrProgram() {
+		if err := m.finalize(mapSpec); err != nil {
+			return nil, fmt.Errorf("finalizing map %s: %w", mapName, err)
+		}
+	}
+
 	cl.maps[mapName] = m
 	return m, nil
 }
@@ -528,44 +537,50 @@ func (cl *collectionLoader) loadProgram(progName string) (*Program, error) {
 	return prog, nil
 }
 
-func (cl *collectionLoader) populateMaps() error {
+// populateDeferredMaps iterates maps holding programs or other maps and loads
+// any dependencies. Populates all maps in cl and freezes them if specified.
+func (cl *collectionLoader) populateDeferredMaps() error {
 	for mapName, m := range cl.maps {
 		mapSpec, ok := cl.coll.Maps[mapName]
 		if !ok {
 			return fmt.Errorf("missing map spec %s", mapName)
 		}
 
+		// Scalar maps without Map or Program references are finalized during
+		// creation. Don't finalize them again.
+		if !mapSpec.Type.canStoreMapOrProgram() {
+			continue
+		}
+
+		mapSpec = mapSpec.Copy()
+
 		// MapSpecs that refer to inner maps or programs within the same
 		// CollectionSpec do so using strings. These strings are used as the key
 		// to look up the respective object in the Maps or Programs fields.
 		// Resolve those references to actual Map or Program resources that
 		// have been loaded into the kernel.
-		if mapSpec.Type.canStoreMap() || mapSpec.Type.canStoreProgram() {
-			mapSpec = mapSpec.Copy()
+		for i, kv := range mapSpec.Contents {
+			objName, ok := kv.Value.(string)
+			if !ok {
+				continue
+			}
 
-			for i, kv := range mapSpec.Contents {
-				objName, ok := kv.Value.(string)
-				if !ok {
-					continue
+			switch t := mapSpec.Type; {
+			case t.canStoreProgram():
+				// loadProgram is idempotent and could return an existing Program.
+				prog, err := cl.loadProgram(objName)
+				if err != nil {
+					return fmt.Errorf("loading program %s, for map %s: %w", objName, mapName, err)
 				}
+				mapSpec.Contents[i] = MapKV{kv.Key, prog}
 
-				switch t := mapSpec.Type; {
-				case t.canStoreProgram():
-					// loadProgram is idempotent and could return an existing Program.
-					prog, err := cl.loadProgram(objName)
-					if err != nil {
-						return fmt.Errorf("loading program %s, for map %s: %w", objName, mapName, err)
-					}
-					mapSpec.Contents[i] = MapKV{kv.Key, prog}
-
-				case t.canStoreMap():
-					// loadMap is idempotent and could return an existing Map.
-					innerMap, err := cl.loadMap(objName)
-					if err != nil {
-						return fmt.Errorf("loading inner map %s, for map %s: %w", objName, mapName, err)
-					}
-					mapSpec.Contents[i] = MapKV{kv.Key, innerMap}
+			case t.canStoreMap():
+				// loadMap is idempotent and could return an existing Map.
+				innerMap, err := cl.loadMap(objName)
+				if err != nil {
+					return fmt.Errorf("loading inner map %s, for map %s: %w", objName, mapName, err)
 				}
+				mapSpec.Contents[i] = MapKV{kv.Key, innerMap}
 			}
 		}
 

types.go

@@ -102,6 +102,12 @@ func (mt MapType) hasPerCPUValue() bool {
 	return mt == PerCPUHash || mt == PerCPUArray || mt == LRUCPUHash || mt == PerCPUCGroupStorage
 }
 
+// canStoreMapOrProgram returns true if the Map stores references to another Map
+// or Program.
+func (mt MapType) canStoreMapOrProgram() bool {
+	return mt.canStoreMap() || mt.canStoreProgram()
+}
+
 // canStoreMap returns true if the map type accepts a map fd
 // for update and returns a map id for lookup.
 func (mt MapType) canStoreMap() bool {