Skip to content

Commit

Permalink
perf - reader.go: document possible garbage read from perf ring, add …
Browse files Browse the repository at this point in the history
…reproducer

A patch for this was submitted upstream:
https://lore.kernel.org/patchwork/patch/1244339.

Promises about the buffer's alignment were also removed because they were
inacurrate. The `perf_event_open` man page is incorrect, and there's
another bug in the kernel that causes these buffers to be unaligned.
  • Loading branch information
ti-mo committed May 19, 2020
1 parent 9a5e0ab commit 7dd9d2a
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 5 deletions.
11 changes: 6 additions & 5 deletions perf/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,8 @@ type Record struct {
CPU int

// The data submitted via bpf_perf_event_output.
// They are padded with 0 to have a 64-bit alignment.
// If you are using variable length samples you need to take
// this into account.
// Due to a kernel bug, this can contain between 0 and 7 bytes of trailing
// garbage from the ring depending on the input sample's length.
RawSample []byte

// The number of samples which could not be output, since
Expand Down Expand Up @@ -317,9 +316,11 @@ func (pr *Reader) Close() error {
// Read the next record from the perf ring buffer.
//
// The function blocks until there are at least Watermark bytes in one
// of the per CPU buffers.
// of the per CPU buffers. Records from buffers below the Watermark
// are not returned.
//
// Records from buffers below the Watermark are not returned.
// Records can contain between 0 and 7 bytes of trailing garbage from the ring
// depending on the input sample's length.
//
// Calling Close interrupts the function.
func (pr *Reader) Read() (Record, error) {
Expand Down
49 changes: 49 additions & 0 deletions perf/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/binary"
"fmt"
"os"
"reflect"
"syscall"
"testing"
"time"
Expand Down Expand Up @@ -390,6 +391,54 @@ func TestPause(t *testing.T) {
}
}

func TestPerfReaderGarbage(t *testing.T) {
// This test reproduces a kernel bug that causes garbage reads from the
// perf ring as soon as the ring's page boundary has wrapped for the first
// time. Sending a 61-byte (unaligned) payload will result in 80 bytes
// written to the perf ring:
// - perf event header (8 bytes)
// - payload (61 + 4 incorrectly added) = 65, aligned to 72

// The 80 bytes total is already aligned to 64 bits, so won't have any
// extra alignment padding when written to the ring. This should cause
// the 52nd write to a 4096-byte ring (4096/80 = 51.2) to wrap the page
// and return 7 `align(61+4)-4 - 61 = 7` bytes of garbage.

// This issue might be fixed in later versions of the kernel.
sl := 61

prog, events := mustOutputSamplesProg(t, sl)
defer prog.Close()
defer events.Close()

rd, err := NewReader(events, 4096)
if err != nil {
t.Fatal(err)
}
defer rd.Close()

for i := 1; i <= 100; i++ {
if _, _, err := prog.Test(make([]byte, 14)); err != nil {
testutils.SkipIfNotSupported(t, err)
t.Fatal(err)
}

record, err := rd.Read()
if err != nil {
t.Fatal("Can't read samples:", err)
}

// Maintain a slice of zeroes the length of the padding section.
nb := make([]byte, len(record.RawSample)-sl)

// Compare the padding section to the empty slice.
if !reflect.DeepEqual(record.RawSample[sl:], nb) {
t.Skipf("%d non-zero bytes found in padding section of perf sample %d: %v", len(nb), i, record.RawSample[sl:])
break
}
}
}

func BenchmarkReader(b *testing.B) {
prog, events := mustOutputSamplesProg(b, 80)
defer prog.Close()
Expand Down

0 comments on commit 7dd9d2a

Please sign in to comment.