Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor out memory allocation #151

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions core/auxiliary.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package core

import (
"os"
"reflect"
"unsafe"
)

Expand All @@ -17,7 +16,10 @@ func roundToPageSize(length int) int {
}

// Convert a pointer and length to a byte slice that describes that memory.
func getBytes(ptr *byte, len int) []byte {
var sl = reflect.SliceHeader{Data: uintptr(unsafe.Pointer(ptr)), Len: len, Cap: len}
return *(*[]byte)(unsafe.Pointer(&sl))
func getBufferPart(buf []byte, offset, length int) []byte {
start := offset
if offset < 0 {
start = len(buf) + offset
}
return unsafe.Slice(&buf[start], length)
}
2 changes: 1 addition & 1 deletion core/auxiliary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestGetBytes(t *testing.T) {
buffer := make([]byte, 32)

// Get am alternate reference to it using our slice builder.
derived := getBytes(&buffer[0], len(buffer))
derived := getBufferPart(buffer, 0, len(buffer))

// Check for naive equality.
if !bytes.Equal(buffer, derived) {
Expand Down
92 changes: 13 additions & 79 deletions core/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package core
import (
"errors"
"sync"

"github.com/awnumar/memcall"
)

var (
buffers = new(bufferList)
allocator = NewPageAllocator()
buffers = new(bufferList)
)

// ErrNullBuffer is returned when attempting to construct a buffer of size less than one.
Expand All @@ -28,66 +27,26 @@ type Buffer struct {
alive bool // Signals that destruction has not come
mutable bool // Mutability state of underlying memory

data []byte // Portion of memory holding the data
memory []byte // Entire allocated memory region

preguard []byte // Guard page addressed before the data
inner []byte // Inner region between the guard pages
postguard []byte // Guard page addressed after the data

canary []byte // Value written behind data to detect spillage
data []byte // Portion of memory holding the data
}

/*
NewBuffer is a raw constructor for the Buffer object.
*/
func NewBuffer(size int) (*Buffer, error) {
var err error

if size < 1 {
return nil, ErrNullBuffer
}

b := new(Buffer)

// Allocate the total needed memory
innerLen := roundToPageSize(size)
b.memory, err = memcall.Alloc((2 * pageSize) + innerLen)
var err error
b.data, err = allocator.Alloc(size)
if err != nil {
Panic(err)
}

// Construct slice reference for data buffer.
b.data = getBytes(&b.memory[pageSize+innerLen-size], size)

// Construct slice references for page sectors.
b.preguard = getBytes(&b.memory[0], pageSize)
b.inner = getBytes(&b.memory[pageSize], innerLen)
b.postguard = getBytes(&b.memory[pageSize+innerLen], pageSize)

// Construct slice reference for canary portion of inner page.
b.canary = getBytes(&b.memory[pageSize], len(b.inner)-len(b.data))

// Lock the pages that will hold sensitive data.
if err := memcall.Lock(b.inner); err != nil {
Panic(err)
}

// Initialise the canary value and reference regions.
if err := Scramble(b.canary); err != nil {
Panic(err)
}
Copy(b.preguard, b.canary)
Copy(b.postguard, b.canary)

// Make the guard pages inaccessible.
if err := memcall.Protect(b.preguard, memcall.NoAccess()); err != nil {
Panic(err)
}
if err := memcall.Protect(b.postguard, memcall.NoAccess()); err != nil {
Panic(err)
}

// Set remaining properties
b.alive = true
b.mutable = true
Expand All @@ -106,7 +65,7 @@ func (b *Buffer) Data() []byte {

// Inner returns a byte slice representing the entire inner memory pages. This should NOT be used unless you have a specific need.
func (b *Buffer) Inner() []byte {
return b.inner
return allocator.Inner(b.data)
}

// Freeze makes the underlying memory of a given buffer immutable. This will do nothing if the Buffer has been destroyed.
Expand All @@ -125,7 +84,7 @@ func (b *Buffer) freeze() error {
}

if b.mutable {
if err := memcall.Protect(b.inner, memcall.ReadOnly()); err != nil {
if err := allocator.Protect(b.data, true); err != nil {
return err
}
b.mutable = false
Expand All @@ -150,7 +109,7 @@ func (b *Buffer) melt() error {
}

if !b.mutable {
if err := memcall.Protect(b.inner, memcall.ReadWrite()); err != nil {
if err := allocator.Protect(b.data, false); err != nil {
return err
}
b.mutable = true
Expand Down Expand Up @@ -198,42 +157,17 @@ func (b *Buffer) destroy() error {
return nil
}

// Make all of the memory readable and writable.
if err := memcall.Protect(b.memory, memcall.ReadWrite()); err != nil {
return err
}
b.mutable = true

// Wipe data field.
Wipe(b.data)

// Verify the canary
if !Equal(b.preguard, b.postguard) || !Equal(b.preguard[:len(b.canary)], b.canary) {
return errors.New("<memguard::core::buffer> canary verification failed; buffer overflow detected")
}

// Wipe the memory.
Wipe(b.memory)

// Unlock pages locked into memory.
if err := memcall.Unlock(b.inner); err != nil {
return err
}

// Free all related memory.
if err := memcall.Free(b.memory); err != nil {
return err
// Destroy the memory content and free the space
if b.data != nil {
if err := allocator.Free(b.data); err != nil {
return err
}
}

// Reset the fields.
b.alive = false
b.mutable = false
b.data = nil
b.memory = nil
b.preguard = nil
b.inner = nil
b.postguard = nil
b.canary = nil
return nil
}

Expand Down
106 changes: 25 additions & 81 deletions core/buffer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,32 @@ import (
"bytes"
"testing"
"unsafe"

"github.com/stretchr/testify/require"
)

func TestNewBuffer(t *testing.T) {
// Check the error case with zero length.
b, err := NewBuffer(0)
if err != ErrNullBuffer {
t.Error("expected ErrNullBuffer; got", err)
}
if b != nil {
t.Error("expected nil buffer; got", b)
}
a, err := NewBuffer(0)
require.ErrorIs(t, err, ErrNullBuffer)
require.Nil(t, a)

// Check the error case with negative length.
b, err = NewBuffer(-1)
if err != ErrNullBuffer {
t.Error("expected ErrNullBuffer; got", err)
}
if b != nil {
t.Error("expected nil buffer; got", b)
}
b, err := NewBuffer(-1)
require.ErrorIs(t, err, ErrNullBuffer)
require.Nil(t, b)

// Test normal execution.
b, err = NewBuffer(32)
if err != nil {
t.Error("expected nil err; got", err)
}
if !b.alive {
t.Error("did not expect destroyed buffer")
}
if len(b.Data()) != 32 || cap(b.Data()) != 32 {
t.Errorf("buffer has invalid length (%d) or capacity (%d)", len(b.Data()), cap(b.Data()))
}
if !b.mutable {
t.Error("buffer is not marked mutable")
}
if len(b.memory) != roundToPageSize(32)+(2*pageSize) {
t.Error("allocated incorrect length of memory")
}
if !bytes.Equal(b.Data(), make([]byte, 32)) {
t.Error("container is not zero-filled")
}
require.NoError(t, err)
require.True(t, b.alive, "did not expect destroyed buffer")
require.Lenf(t, b.Data(), 32, "buffer has invalid length (%d)", len(b.Data()))
require.Equalf(t, cap(b.Data()), 32, "buffer has invalid capacity (%d)", cap(b.Data()))
require.True(t, b.mutable, "buffer is not marked mutable")
require.EqualValues(t, make([]byte, 32), b.Data(), "container is not zero-filled")

// Check if the buffer was added to the buffers list.
if !buffers.exists(b) {
t.Error("buffer not in buffers list")
}
require.True(t, buffers.exists(b), "buffer not in buffers list")

// Destroy the buffer.
b.Destroy()
Expand All @@ -58,35 +38,17 @@ func TestNewBuffer(t *testing.T) {
func TestLotsOfAllocs(t *testing.T) {
for i := 1; i <= 16385; i++ {
b, err := NewBuffer(i)
if err != nil {
t.Error(err)
}
if !b.alive || !b.mutable {
t.Error("invalid metadata")
}
if len(b.data) != i {
t.Error("invalid data length")
}
if len(b.memory) != roundToPageSize(i)+2*pageSize {
t.Error("memory length invalid")
}
if len(b.preguard) != pageSize || len(b.postguard) != pageSize {
t.Error("guard pages length invalid")
}
if len(b.canary) != len(b.inner)-i {
t.Error("canary length invalid")
}
if len(b.inner)%pageSize != 0 {
t.Error("inner length is not multiple of page size")
}
require.NoErrorf(t, err, "creating buffer in iteration %d", i)
require.Truef(t, b.alive, "not alive in iteration %d", i)
require.Truef(t, b.mutable, "not mutable in iteration %d", i)
require.Lenf(t, b.data, i, "invalid data length %d in iteration %d", len(b.data), i)
require.Zerof(t, len(b.Inner())%pageSize, "inner length %d is not multiple of page size in iteration %d", len(b.Inner()), i)

// Fill data
for j := range b.data {
b.data[j] = 1
}
for j := range b.data {
if b.data[j] != 1 {
t.Error("region rw test failed")
}
}
require.Equalf(t, bytes.Repeat([]byte{1}, i), b.data, "region rw test failed in iteration %d", i)
b.Destroy()
}
}
Expand Down Expand Up @@ -184,21 +146,12 @@ func TestDestroy(t *testing.T) {
if b.Data() != nil {
t.Error("expected bytes buffer to be nil; got", b.Data())
}
if b.memory != nil {
t.Error("expected memory to be nil; got", b.memory)
}
if b.mutable || b.alive {
t.Error("buffer should be dead and immutable")
}
if b.preguard != nil || b.postguard != nil {
t.Error("guard page slice references are not nil")
}
if b.inner != nil {
if b.Inner() != nil {
t.Error("inner pages slice reference not nil")
}
if b.canary != nil {
t.Error("canary slice reference not nil")
}

// Check if the buffer was removed from the buffers list.
if buffers.exists(b) {
Expand All @@ -212,21 +165,12 @@ func TestDestroy(t *testing.T) {
if b.Data() != nil {
t.Error("expected bytes buffer to be nil; got", b.Data())
}
if b.memory != nil {
t.Error("expected memory to be nil; got", b.memory)
}
if b.mutable || b.alive {
t.Error("buffer should be dead and immutable")
}
if b.preguard != nil || b.postguard != nil {
t.Error("guard page slice references are not nil")
}
if b.inner != nil {
if b.Inner() != nil {
t.Error("inner pages slice reference not nil")
}
if b.canary != nil {
t.Error("canary slice reference not nil")
}
}

func TestBufferList(t *testing.T) {
Expand Down
14 changes: 0 additions & 14 deletions core/exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package core
import (
"fmt"
"os"

"github.com/awnumar/memcall"
)

/*
Expand Down Expand Up @@ -37,18 +35,6 @@ func Purge() {
} else {
opErr = fmt.Errorf("%s; %s", opErr.Error(), err.Error())
}
// buffer destroy failed; wipe instead
awnumar marked this conversation as resolved.
Show resolved Hide resolved
b.Lock()
defer b.Unlock()
if !b.mutable {
if err := memcall.Protect(b.inner, memcall.ReadWrite()); err != nil {
// couldn't change it to mutable; we can't wipe it! (could this happen?)
// not sure what we can do at this point, just warn and move on
fmt.Fprintf(os.Stderr, "!WARNING: failed to wipe immutable data at address %p", &b.data)
continue // wipe in subprocess?
}
}
Wipe(b.data)
}
}
}()
Expand Down
2 changes: 1 addition & 1 deletion core/exit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestPurge(t *testing.T) {
if err != nil {
t.Error(err)
}
Scramble(b.inner)
Scramble(allocator.Inner(b.data))
b.Freeze()
if !panics(func() {
Purge()
Expand Down
Loading