map: unmarshaling silently ignores too small keys or values

[ti-mo]

Describe the bug
Map.BatchLookup fails silently if nextKey is undersized, leaving nextKey with a zero value.

To Reproduce

package main

import (
	"errors"
	"log"

	"github.com/cilium/ebpf"
)

func main() {
	m, err := ebpf.NewMap(&ebpf.MapSpec{
		Type:       ebpf.LRUHash,
		KeySize:    8,
		ValueSize:  8,
		MaxEntries: 1_024_000,
	})
	if err != nil {
		log.Fatal(err)
	}
	defer m.Close()

	keys, values := make([]uint64, m.MaxEntries()), make([]uint64, m.MaxEntries())
	for i := 0; uint32(i) < m.MaxEntries(); i++ {
		keys[i] = uint64(i)
		values[i] = uint64(i)
	}

	count, err := m.BatchUpdate(keys, values, nil)
	if err != nil {
		log.Fatalf("BatchUpdate: %s", err)
	}
	log.Println("wrote", count, "entries")

	// Undersized nextKey. Only uint64 should be accepted here. (KeySize 8)
	// Should contain 1048576 at the end of program. uint32 works as well, as it
	// can represent up to 4.2B.
	var nextKey uint16

	keysOut, valuesOut := make([]uint64, m.MaxEntries()), make([]uint64, m.MaxEntries())
	count, err = m.BatchLookup(nil, &nextKey, keysOut, valuesOut, nil)
	if err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) {
		log.Fatalf("BatchLookup: %s", err)
	}

	log.Printf("count: %d, nextKey: %d", count, nextKey)
}

Expected behavior

BatchLookup should fail with an unmarshaling error.

[devidasjadhav]

I can reproduced this.
Some of my findings:

1. ebpf/map.go

   Line 919 in 3a926eb

   func (m *Map) BatchLookup(prevKey, nextKeyOut, keysOut, valuesOut interface{}, opts *BatchOptions) (int, error) {

nextKeyOut has no explicit type.

2. As we are passing generic pointer so we cannot test this against m.keySize.

@ti-mo
Do you have any pointers on this ?

[lmb]

I'll see whether this was introduced by #1062.

P.S. Doesn't seem to be caused by the marshaling changes.

diff --git a/map_test.go b/map_test.go
index 2d3e4add..6d50e685 100644
--- a/map_test.go
+++ b/map_test.go
@@ -227,6 +227,36 @@ func TestBatchAPIHash(t *testing.T) {
 	}
 }
 
+func TestBatchLookupKeyTooSmall(t *testing.T) {
+	m, err := NewMap(&MapSpec{
+		Type:       LRUHash,
+		KeySize:    8,
+		ValueSize:  8,
+		MaxEntries: 10,
+	})
+	qt.Assert(t, err, qt.IsNil)
+	defer m.Close()
+
+	keys, values := make([]uint64, m.MaxEntries()), make([]uint64, m.MaxEntries())
+	for i := 0; uint32(i) < m.MaxEntries(); i++ {
+		keys[i] = uint64(i)
+		values[i] = uint64(i)
+	}
+
+	count, err := m.BatchUpdate(keys, values, nil)
+	qt.Assert(t, err, qt.IsNil)
+	t.Log("wrote", count, "entries")
+
+	// Undersized nextKey. Only uint64 should be accepted here. (KeySize 8)
+	// Should contain 1048576 at the end of program. uint32 works as well, as it
+	// can represent up to 4.2B.
+	var nextKey uint16
+
+	keysOut, valuesOut := make([]uint64, m.MaxEntries()), make([]uint64, m.MaxEntries())
+	_, err = m.BatchLookup(nil, &nextKey, keysOut, valuesOut, nil)
+	qt.Assert(t, err, qt.IsNotNil)
+}
+
 func TestBatchAPIMapDelete(t *testing.T) {
 	if err := haveBatchAPI(); err != nil {
 		t.Skipf("batch api not available: %v", err)

[lmb]

The problem is here somewhere most likely:

ebpf/map.go

Lines 985 to 993 in 9598c01

	sysErr = wrapMapError(sysErr)
	if sysErr != nil && !errors.Is(sysErr, unix.ENOENT) {
	return 0, sysErr
	}
	err = nextBuf.Unmarshal(nextKeyOut)
	if err != nil {
	return 0, err
	}

[lmb]

This is actually not batch specific, it applies to all unmarshaling from map values.