From e83be32c0b1bc708f55733404cc97a76a42b9520 Mon Sep 17 00:00:00 2001 From: Lorenz Bauer Date: Fri, 20 Oct 2023 10:45:26 +0100 Subject: [PATCH] sysenc: special case unmarshaling into []byte The library has somewhat complicated semantics when unmarshaling map keys and values. It turns out that the following was supported (but not documented) until the zero-allocation marshaling work broke it: var s []byte m.Lookup(key, &s) The important thing is that the slice here is empty. The current code treats this as "the key should be zero length" while originally we'd assign the temporary buffer used by the syscall to s as a way to reduce allocations. In hindsight this wasn't a great idea, but it is what it is. Reintroduce the byte slice special case. Fixes https://github.com/cilium/ebpf/issues/1175 Signed-off-by: Lorenz Bauer --- internal/sysenc/marshal.go | 7 +++++++ map.go | 6 +----- map_test.go | 4 ++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/internal/sysenc/marshal.go b/internal/sysenc/marshal.go index 235a1df26..52056dff6 100644 --- a/internal/sysenc/marshal.go +++ b/internal/sysenc/marshal.go @@ -11,6 +11,8 @@ import ( "unsafe" "github.com/cilium/ebpf/internal" + + "golang.org/x/exp/slices" ) // Marshal turns data into a byte slice using the system's native endianness. @@ -88,6 +90,11 @@ func Unmarshal(data interface{}, buf []byte) error { *value = string(buf) return nil + case *[]byte: + // Backwards compat: unmarshaling into a slice replaces the whole slice. + *value = slices.Clone(buf) + return nil + default: if dataBuf := unsafeBackingMemory(data); len(dataBuf) == len(buf) { copy(dataBuf, buf) diff --git a/map.go b/map.go index be732a24f..7f6184850 100644 --- a/map.go +++ b/map.go @@ -1411,11 +1411,7 @@ func (mi *MapIterator) Next(keyOut, valueOut interface{}) bool { return false } - // The user can get access to nextKey since unmarshalBytes - // does not copy when unmarshaling into a []byte. - // Make a copy to prevent accidental corruption of - // iterator state. - copy(mi.curKey, nextKey) + mi.curKey = nextKey mi.count++ mi.err = mi.target.Lookup(nextKey, valueOut) diff --git a/map_test.go b/map_test.go index ae395320b..165434aa4 100644 --- a/map_test.go +++ b/map_test.go @@ -75,6 +75,10 @@ func TestMap(t *testing.T) { t.Error("Want value 42, got", v) } + var slice []byte + qt.Assert(t, m.Lookup(uint32(0), &slice), qt.IsNil) + qt.Assert(t, slice, qt.DeepEquals, internal.NativeEndian.AppendUint32(nil, 42)) + var k uint32 if err := m.NextKey(uint32(0), &k); err != nil { t.Fatal("Can't get:", err)