Skip to content

Commit

Permalink
sysenc: special case unmarshaling into []byte
Browse files Browse the repository at this point in the history
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 #1175

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
  • Loading branch information
lmb committed Oct 20, 2023
1 parent 73d68a3 commit e83be32
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 5 deletions.
7 changes: 7 additions & 0 deletions internal/sysenc/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 1 addition & 5 deletions map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit e83be32

Please sign in to comment.