diff --git a/cmd/bpf2go/gen/types.go b/cmd/bpf2go/gen/types.go index f1413c1bf..93409e20d 100644 --- a/cmd/bpf2go/gen/types.go +++ b/cmd/bpf2go/gen/types.go @@ -44,12 +44,7 @@ func collectMapTypes(types []btf.Type, maps map[string]*ebpf.MapSpec) []btf.Type // collectVariableTypes collects all types used by VariableSpecs. func collectVariableTypes(types []btf.Type, vars map[string]*ebpf.VariableSpec) []btf.Type { for _, vs := range vars { - v := vs.Type() - if v == nil { - continue - } - - types = addType(types, v.Type) + types = addType(types, vs.Type.Type) } return types diff --git a/collection.go b/collection.go index 36c6de258..58fdb260e 100644 --- a/collection.go +++ b/collection.go @@ -73,7 +73,7 @@ func (cs *CollectionSpec) Copy() *CollectionSpec { } for name, spec := range cs.Variables { - cpy.Variables[name] = spec.copy(&cpy) + cpy.Variables[name] = spec.Copy() } if cs.Variables == nil { cpy.Variables = nil @@ -170,7 +170,7 @@ func (cs *CollectionSpec) RewriteConstants(consts map[string]interface{}) error continue } - if !v.Constant() { + if !strings.HasPrefix(v.MapName, ".rodata") { return fmt.Errorf("variable %s is not a constant", n) } @@ -514,6 +514,19 @@ func (cl *collectionLoader) loadMap(mapName string) (*Map, error) { return m, nil } + var vars []*VariableSpec + for _, vs := range cl.coll.Variables { + if vs.MapName != mapName { + continue + } + + vars = append(vars, vs) + } + + if err := mapSpec.updateDataSection(vars); err != nil { + return nil, fmt.Errorf("assembling contents: %w", err) + } + m, err := newMapWithOptions(mapSpec, cl.opts.Maps, cl.types) if err != nil { return nil, fmt.Errorf("map %s: %w", mapName, err) @@ -597,19 +610,7 @@ func (cl *collectionLoader) loadVariable(varName string) (*Variable, error) { return nil, fmt.Errorf("unknown variable %s", varName) } - // Get the key of the VariableSpec's MapSpec in the CollectionSpec. - var mapName string - for n, ms := range cl.coll.Maps { - if ms == varSpec.m { - mapName = n - break - } - } - if mapName == "" { - return nil, fmt.Errorf("variable %s: underlying MapSpec %s was removed from CollectionSpec", varName, varSpec.m.Name) - } - - m, err := cl.loadMap(mapName) + m, err := cl.loadMap(varSpec.MapName) if err != nil { return nil, fmt.Errorf("variable %s: %w", varName, err) } @@ -626,14 +627,14 @@ func (cl *collectionLoader) loadVariable(varName string) (*Variable, error) { mm, err = m.Memory() } if err != nil && !errors.Is(err, ErrNotSupported) { - return nil, fmt.Errorf("variable %s: getting memory for map %s: %w", varName, mapName, err) + return nil, fmt.Errorf("variable %s: getting memory for map %s: %w", varName, varSpec.MapName, err) } v, err := newVariable( - varSpec.name, - varSpec.offset, - varSpec.size, - varSpec.t, + varSpec.Name, + varSpec.Offset, + len(varSpec.Value), + varSpec.Type, mm, ) if err != nil { @@ -715,9 +716,9 @@ func (cl *collectionLoader) populateStructOps(m *Map, mapSpec *MapSpec) error { return fmt.Errorf("value should be a *Struct") } - userData, ok := mapSpec.Contents[0].Value.([]byte) - if !ok { - return fmt.Errorf("value should be an array of byte") + userData, err := mapSpec.dataSection() + if err != nil { + return fmt.Errorf("getting data section: %w", err) } if len(userData) < int(userType.Size) { return fmt.Errorf("user data too short: have %d, need at least %d", len(userData), userType.Size) diff --git a/collection_test.go b/collection_test.go index b0b32838b..8b6008aa7 100644 --- a/collection_test.go +++ b/collection_test.go @@ -91,10 +91,9 @@ func TestCollectionSpecCopy(t *testing.T) { }, map[string]*VariableSpec{ "my-var": { - name: "my-var", - offset: 0, - size: 4, - m: ms, + Name: "my-var", + MapName: "my-map", + Offset: 0, }, }, &btf.Spec{}, diff --git a/elf_reader.go b/elf_reader.go index f2c9196b7..68b6c3948 100644 --- a/elf_reader.go +++ b/elf_reader.go @@ -1210,14 +1210,15 @@ func (ec *elfCode) loadDataSections() error { mapSpec.Flags = sys.BPF_F_RDONLY_PROG } + var data []byte switch sec.Type { // Only open the section if we know there's actual data to be read. case elf.SHT_PROGBITS: - data, err := sec.Data() + var err error + data, err = sec.Data() if err != nil { return fmt.Errorf("data section %s: can't get contents: %w", sec.Name, err) } - mapSpec.Contents = []MapKV{{uint32(0), data}} case elf.SHT_NOBITS: // NOBITS sections like .bss contain only zeroes and are not allocated in @@ -1225,12 +1226,14 @@ func (ec *elfCode) loadDataSections() error { // them. Don't attempt reading zeroes from the ELF, instead allocate the // zeroed memory to support getting and setting VariableSpecs for sections // like .bss. - mapSpec.Contents = []MapKV{{uint32(0), make([]byte, sec.Size)}} + data = make([]byte, sec.Size) default: return fmt.Errorf("data section %s: unknown section type %s", sec.Name, sec.Type) } + mapSpec.Contents = []MapKV{{uint32(0), data}} + for off, sym := range sec.symbols { // Skip symbols marked with the 'hidden' attribute. if elf.ST_VISIBILITY(sym.Other) == elf.STV_HIDDEN || @@ -1259,11 +1262,19 @@ func (ec *elfCode) loadDataSections() error { continue } + if off+sym.Size > uint64(len(data)) { + return fmt.Errorf("data section %s: variable %s exceeds section bounds", sec.Name, sym.Name) + } + + if off > math.MaxInt { + return fmt.Errorf("data section %s: variable %s offset %d exceeds maximum", sec.Name, sym.Name, off) + } + ec.vars[sym.Name] = &VariableSpec{ - name: sym.Name, - offset: off, - size: sym.Size, - m: mapSpec, + MapName: sec.Name, + Name: sym.Name, + Offset: int(off), + Value: slices.Clone(data[off : off+sym.Size]), } } @@ -1294,17 +1305,17 @@ func (ec *elfCode) loadDataSections() error { continue } - if uint64(v.Offset) != ev.offset { - return fmt.Errorf("data section %s: variable %s datasec offset (%d) doesn't match ELF symbol offset (%d)", sec.Name, name, v.Offset, ev.offset) + if v.Offset != uint32(ev.Offset) { + return fmt.Errorf("data section %s: variable %s datasec offset (%d) doesn't match ELF symbol offset (%d)", sec.Name, name, v.Offset, ev.Offset) } - if uint64(v.Size) != ev.size { - return fmt.Errorf("data section %s: variable %s size in datasec (%d) doesn't match ELF symbol size (%d)", sec.Name, name, v.Size, ev.size) + if v.Size != uint32(len(ev.Value)) { + return fmt.Errorf("data section %s: variable %s size in datasec (%d) doesn't match ELF symbol size (%d)", sec.Name, name, v.Size, len(ev.Value)) } // Decouple the Var in the VariableSpec from the underlying DataSec in // the MapSpec to avoid modifications from affecting map loads later on. - ev.t = btf.Copy(vt).(*btf.Var) + ev.Type = btf.Copy(vt).(*btf.Var) } } } diff --git a/elf_reader_test.go b/elf_reader_test.go index 9b4fcf4aa..246ff3510 100644 --- a/elf_reader_test.go +++ b/elf_reader_test.go @@ -32,16 +32,11 @@ var csCmpOpts = cmp.Options{ } return false }), - cmp.Comparer(func(a, b *VariableSpec) bool { - if a.name != b.name || a.offset != b.offset || a.size != b.size { - return false - } - return true - }), cmpopts.IgnoreTypes(btf.Spec{}), cmpopts.IgnoreFields(CollectionSpec{}, "ByteOrder", "Types"), cmpopts.IgnoreFields(ProgramSpec{}, "Instructions", "ByteOrder"), cmpopts.IgnoreFields(MapSpec{}, "Key", "Value", "Contents"), + cmpopts.IgnoreFields(VariableSpec{}, "Type", "Value"), cmpopts.IgnoreUnexported(ProgramSpec{}), } @@ -224,14 +219,14 @@ func TestLoadCollectionSpec(t *testing.T) { }, }, Variables: map[string]*VariableSpec{ - "arg": {name: "arg", offset: 4, size: 4}, - "arg2": {name: "arg2", offset: 0, size: 4}, - "arg3": {name: "arg3", offset: 0, size: 4}, - "key1": {name: "key1", offset: 0, size: 4}, - "key2": {name: "key2", offset: 0, size: 4}, - "key3": {name: "key3", offset: 0, size: 4}, - "neg": {name: "neg", offset: 12, size: 4}, - "uneg": {name: "uneg", offset: 8, size: 4}, + "arg": {Name: "arg", MapName: ".rodata", Offset: 4}, + "arg2": {Name: "arg2", MapName: ".rodata.test", Offset: 0}, + "arg3": {Name: "arg3", MapName: ".data.test", Offset: 0}, + "key1": {Name: "key1", MapName: ".bss", Offset: 0}, + "key2": {Name: "key2", MapName: ".data", Offset: 0}, + "key3": {Name: "key3", MapName: ".rodata", Offset: 0}, + "neg": {Name: "neg", MapName: ".rodata", Offset: 12}, + "uneg": {Name: "uneg", MapName: ".rodata", Offset: 8}, }, } diff --git a/helpers_test.go b/helpers_test.go index b9169b660..52ad98f48 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -365,9 +365,5 @@ func fixupCollectionSpec(spec *CollectionSpec) *CollectionSpec { spec.Programs[name] = fixupProgramSpec(spec.Programs[name]) } - for _, varSpec := range spec.Variables { - varSpec.m = spec.Maps[varSpec.m.Name] - } - return spec } diff --git a/map.go b/map.go index f9499272b..5cd18e3fd 100644 --- a/map.go +++ b/map.go @@ -10,6 +10,7 @@ import ( "path/filepath" "reflect" "slices" + "sort" "strings" "sync" "time" @@ -29,7 +30,6 @@ var ( ErrKeyExist = errors.New("key already exists") ErrIterationAborted = errors.New("iteration aborted") ErrMapIncompatible = errors.New("map spec is incompatible with existing map") - errMapNoBTFValue = errors.New("map spec does not contain a BTF Value") // pre-allocating these errors here since they may get called in hot code paths // and cause unnecessary memory allocations @@ -188,27 +188,61 @@ func (spec *MapSpec) fixupMagicFields() (*MapSpec, error) { } // dataSection returns the contents and BTF Datasec descriptor of the spec. -func (ms *MapSpec) dataSection() ([]byte, *btf.Datasec, error) { - if ms.Value == nil { - return nil, nil, errMapNoBTFValue +func (ms *MapSpec) dataSection() ([]byte, error) { + if n := len(ms.Contents); n != 1 { + return nil, fmt.Errorf("expected one key, found %d", n) + } + + kv := ms.Contents[0] + if key, ok := ms.Contents[0].Key.(uint32); !ok || key != 0 { + return nil, fmt.Errorf("expected contents to have key 0") } - ds, ok := ms.Value.(*btf.Datasec) + value, ok := kv.Value.([]byte) if !ok { - return nil, nil, fmt.Errorf("map value BTF is a %T, not a *btf.Datasec", ms.Value) + return nil, fmt.Errorf("value at first map key is %T, not []byte", kv.Value) } - if n := len(ms.Contents); n != 1 { - return nil, nil, fmt.Errorf("expected one key, found %d", n) + return value, nil +} + +// updateDataSection copies the values of vars into MapSpec.Contents[0].Value. +// +// vars is sorted in place by offset. +func (ms *MapSpec) updateDataSection(vars []*VariableSpec) error { + if len(vars) == 0 { + return nil } - kv := ms.Contents[0] - value, ok := kv.Value.([]byte) - if !ok { - return nil, nil, fmt.Errorf("value at first map key is %T, not []byte", kv.Value) + data, err := ms.dataSection() + if err != nil { + return err + } + + // Do not modify the original data slice, ms.Contents is a shallow copy. + data = slices.Clone(data) + + sort.Slice(vars, func(i, j int) bool { + return vars[i].Offset < vars[j].Offset + }) + + offset := 0 + for _, v := range vars { + if v.Offset < offset { + return fmt.Errorf("variable %s overlaps with previous variable", v.Name) + } + + end := v.Offset + len(v.Value) + if end > len(data) { + return fmt.Errorf("variable %s exceeds map size", v.Name) + } + + copy(data[v.Offset:end], v.Value) + offset = end } - return value, ds, nil + ms.Contents = []MapKV{{Key: uint32(0), Value: data}} + return nil } func (ms *MapSpec) readOnly() bool { diff --git a/variable.go b/variable.go index c6fd55cba..fc150d63e 100644 --- a/variable.go +++ b/variable.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "reflect" + "slices" "github.com/cilium/ebpf/btf" "github.com/cilium/ebpf/internal/sysenc" @@ -15,118 +16,48 @@ import ( // All operations on a VariableSpec's underlying MapSpec are performed in the // host's native endianness. type VariableSpec struct { - name string - offset uint64 - size uint64 - - m *MapSpec - t *btf.Var + Name string + // Name of the map this variable belongs to. + MapName string + // Offset of the variable within the map. + Offset int + // Byte representation of the variables's value. + Value []byte + // Type information of the variable. Optional. + Type *btf.Var } // Set sets the value of the VariableSpec to the provided input using the host's // native endianness. func (s *VariableSpec) Set(in any) error { - buf, err := sysenc.Marshal(in, int(s.size)) - if err != nil { - return fmt.Errorf("marshaling value %s: %w", s.name, err) - } - - b, _, err := s.m.dataSection() + buf, err := sysenc.Marshal(in, len(s.Value)) if err != nil { - return fmt.Errorf("getting data section of map %s: %w", s.m.Name, err) + return fmt.Errorf("marshaling value %s: %w", s.Name, err) } - if int(s.offset+s.size) > len(b) { - return fmt.Errorf("offset %d(+%d) for variable %s is out of bounds", s.offset, s.size, s.name) - } - - // MapSpec.Copy() performs a shallow copy. Fully copy the byte slice - // to avoid any changes affecting other copies of the MapSpec. - cpy := make([]byte, len(b)) - copy(cpy, b) - - buf.CopyTo(cpy[s.offset : s.offset+s.size]) - - s.m.Contents[0] = MapKV{Key: uint32(0), Value: cpy} - + buf.CopyTo(s.Value) return nil } // Get writes the value of the VariableSpec to the provided output using the // host's native endianness. func (s *VariableSpec) Get(out any) error { - b, _, err := s.m.dataSection() - if err != nil { - return fmt.Errorf("getting data section of map %s: %w", s.m.Name, err) - } - - if int(s.offset+s.size) > len(b) { - return fmt.Errorf("offset %d(+%d) for variable %s is out of bounds", s.offset, s.size, s.name) - } - - if err := sysenc.Unmarshal(out, b[s.offset:s.offset+s.size]); err != nil { + if err := sysenc.Unmarshal(out, s.Value); err != nil { return fmt.Errorf("unmarshaling value: %w", err) } return nil } -// Size returns the size of the variable in bytes. -func (s *VariableSpec) Size() uint64 { - return s.size -} - -// MapName returns the name of the underlying MapSpec. -func (s *VariableSpec) MapName() string { - return s.m.Name -} - -// Offset returns the offset of the variable in the underlying MapSpec. -func (s *VariableSpec) Offset() uint64 { - return s.offset -} - -// Constant returns true if the VariableSpec represents a variable that is -// read-only from the perspective of the BPF program. -func (s *VariableSpec) Constant() bool { - return s.m.readOnly() -} - -// Type returns the [btf.Var] representing the variable in its data section. -// This is useful for inspecting the variable's decl tags and the type -// information of the inner type. -// -// Returns nil if the original ELF object did not contain BTF information. -func (s *VariableSpec) Type() *btf.Var { - return s.t -} - func (s *VariableSpec) String() string { - return fmt.Sprintf("%s (type=%v, map=%s, offset=%d, size=%d)", s.name, s.t, s.m.Name, s.offset, s.size) + return fmt.Sprintf("%s (type=%v, map=%s)", s.Name, s.Type, s.MapName) } -// copy returns a new VariableSpec with the same values as the original, -// but with a different underlying MapSpec. This is useful when copying a -// CollectionSpec. Returns nil if a MapSpec with the same name is not found. -func (s *VariableSpec) copy(cpy *CollectionSpec) *VariableSpec { - out := &VariableSpec{ - name: s.name, - offset: s.offset, - size: s.size, - } - if s.t != nil { - out.t = btf.Copy(s.t).(*btf.Var) - } - - // Attempt to find a MapSpec with the same name in the copied CollectionSpec. - for _, m := range cpy.Maps { - if m.Name == s.m.Name { - out.m = m - return out - } - } - - return nil +// Copy the VariableSpec. +func (s *VariableSpec) Copy() *VariableSpec { + cpy := *s + cpy.Value = slices.Clone(s.Value) + return &cpy } // Variable is a convenience wrapper for modifying global variables of a @@ -144,17 +75,17 @@ type Variable struct { mm *Memory } -func newVariable(name string, offset, size uint64, t *btf.Var, mm *Memory) (*Variable, error) { +func newVariable(name string, offset, size int, t *btf.Var, mm *Memory) (*Variable, error) { if mm != nil { - if int(offset+size) > mm.Size() { + if offset+size > mm.Size() { return nil, fmt.Errorf("offset %d(+%d) is out of bounds", offset, size) } } return &Variable{ name: name, - offset: offset, - size: size, + offset: uint64(offset), + size: uint64(size), t: t, mm: mm, }, nil diff --git a/variable_test.go b/variable_test.go index e740ef8d1..f571036d3 100644 --- a/variable_test.go +++ b/variable_test.go @@ -62,7 +62,7 @@ func TestVariableSpecCopy(t *testing.T) { const want uint32 = 0xfefefefe wantb := []byte{0xfe, 0xfe, 0xfe, 0xfe} // Same byte sequence regardless of endianness qt.Assert(t, qt.IsNil(cpy.Variables["var_rodata"].Set(want))) - qt.Assert(t, qt.DeepEquals(cpy.Maps[".rodata"].Contents[0].Value.([]byte), wantb)) + qt.Assert(t, qt.DeepEquals(cpy.Variables["var_rodata"].Value, wantb)) // Verify that the original underlying MapSpec was not modified. zero := make([]byte, 4) @@ -70,8 +70,8 @@ func TestVariableSpecCopy(t *testing.T) { // Check that modifications to the VariableSpec's Type don't affect the // underlying MapSpec's type information on either the original or the copy. - cpy.Variables["var_rodata"].Type().Name = "modified" - spec.Variables["var_rodata"].Type().Name = "modified" + cpy.Variables["var_rodata"].Type.Name = "modified" + spec.Variables["var_rodata"].Type.Name = "modified" qt.Assert(t, qt.Equals(cpy.Maps[".rodata"].Value.(*btf.Datasec).Vars[0].Type.(*btf.Var).Name, "var_rodata")) qt.Assert(t, qt.Equals(spec.Maps[".rodata"].Value.(*btf.Datasec).Vars[0].Type.(*btf.Var).Name, "var_rodata"))