map: remove MapSpec.Freeze field

This has been an ambiguous flag since its inception. Internally, we would allow
maps to be created with BPF_F_RDONLY_PROG, but not freeze them afterwards.
Conversely, we would allow MapSpecs with Freeze set to 'true', but without the
rdonly map flag set.

The latter case has caused subtle bugs in the past, since BPF_MAP_FREEZE only
blocks further modifications from user space, but doesn't actually mark the map
as readonly for the verifier. This will prevent code pruning and other optimizations
from taking place.

This commit removes the Freeze field in favor of the map flag and adds a helper to
keep existing call sites simple. Sourcegraph code search yields no references to
this field in open-source code.

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

elf_reader.go

@@ -200,6 +200,10 @@ func loadVersion(sec *elf.Section, bo binary.ByteOrder) (uint32, error) {
 	return version, nil
 }
 
+func isConstantDataSection(name string) bool {
+	return strings.HasPrefix(name, ".rodata")
+}
+
 type elfSectionKind int
 
 const (
@@ -1137,9 +1141,8 @@ func (ec *elfCode) loadDataSections() error {
 			}
 		}
 
-		if strings.HasPrefix(sec.Name, ".rodata") {
+		if isConstantDataSection(sec.Name) {
 			mapSpec.Flags = sys.BPF_F_RDONLY_PROG
-			mapSpec.Freeze = true
 		}
 
 		ec.maps[sec.Name] = mapSpec
@@ -1175,7 +1178,6 @@ func (ec *elfCode) loadKconfigSection() error {
 		ValueSize:  ds.Size,
 		MaxEntries: 1,
 		Flags:      sys.BPF_F_RDONLY_PROG,
-		Freeze:     true,
 		Key:        &btf.Int{Size: 4},
 		Value:      ds,
 	}

elf_reader_test.go

@@ -491,7 +491,7 @@ func TestStringSection(t *testing.T) {
 		t.Fatal("Unable to find map '.rodata.str1.1' in loaded collection")
 	}
 
-	if !strMap.Freeze {
+	if !strMap.readOnly() {
 		t.Fatal("Read only data maps should be frozen")
 	}
 

map.go

@@ -73,9 +73,6 @@ type MapSpec struct {
 	// The initial contents of the map. May be nil.
 	Contents []MapKV
 
-	// Whether to freeze a map after setting its initial contents.
-	Freeze bool
-
 	// InnerMap is used as a template for ArrayOfMaps and HashOfMaps
 	InnerMap *MapSpec
 
@@ -190,6 +187,10 @@ func (ms *MapSpec) dataSection() ([]byte, *btf.Datasec, error) {
 	return value, ds, nil
 }
 
+func (ms *MapSpec) readOnly() bool {
+	return (ms.Flags & sys.BPF_F_RDONLY_PROG) > 0
+}
+
 // MapKV is used to initialize the contents of a Map.
 type MapKV struct {
 	Key   interface{}
@@ -484,7 +485,7 @@ func handleMapCreateError(attr sys.MapCreateAttr, spec *MapSpec, err error) erro
 			return fmt.Errorf("map create: %w", haveFeatErr)
 		}
 	}
-	if spec.Flags&(sys.BPF_F_RDONLY_PROG|sys.BPF_F_WRONLY_PROG) > 0 || spec.Freeze {
+	if spec.Flags&(sys.BPF_F_RDONLY_PROG|sys.BPF_F_WRONLY_PROG) > 0 {
 		if haveFeatErr := haveMapMutabilityModifiers(); haveFeatErr != nil {
 			return fmt.Errorf("map create: %w", haveFeatErr)
 		}
@@ -1400,7 +1401,7 @@ func (m *Map) finalize(spec *MapSpec) error {
 		}
 	}
 
-	if spec.Freeze {
+	if spec.readOnly() {
 		if err := m.Freeze(); err != nil {
 			return fmt.Errorf("freezing map: %w", err)
 		}

map_test.go

@@ -104,8 +104,7 @@ func TestMapSpecCopy(t *testing.T) {
 		PinByName,
 		1,
 		[]MapKV{{1, 2}}, // Can't copy Contents, use value types
-		true,
-		nil, // InnerMap
+		nil,             // InnerMap
 		bytes.NewReader(nil),
 		&btf.Int{},
 		&btf.Int{},