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

ipc: fix endianness issues #1854

Merged
merged 1 commit into from
Jun 23, 2020
Merged
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
19 changes: 13 additions & 6 deletions executor/executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ const int kOutFd = 4;
static uint32* output_data;
static uint32* output_pos;
static uint32* write_output(uint32 v);
static uint32* write_output_64(uint64 v);
Copy link
Contributor Author

@eaibmz eaibmz Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the name. Maybe we should use function overloading here but i decided against because
it is easy to misuse and pass uint32 instead of uint64 and compiler will not warn about it. What do you think ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with this version for now. I don't see significant reasons to switch to something else now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

static void write_completed(uint32 completed);
static uint32 hash(uint32 a);
static bool dedup(uint32 sig);
Expand Down Expand Up @@ -1308,6 +1309,15 @@ uint32* write_output(uint32 v)
return output_pos++;
}

uint32* write_output_64(uint64 v)
{
if (output_pos < output_data || (char*)(output_pos + 1) >= (char*)output_data + kMaxOutput)
fail("output overflow: pos=%p region=[%p:%p]",
output_pos, output_data, (char*)output_data + kMaxOutput);
*(uint64*)output_pos = v;
return output_pos + 2;
}

void write_completed(uint32 completed)
{
__atomic_store_n(output_data, completed, __ATOMIC_RELEASE);
Expand Down Expand Up @@ -1344,13 +1354,10 @@ void kcov_comparison_t::write()
if (!is_size_8) {
write_output((uint32)arg1);
write_output((uint32)arg2);
return;
} else {
write_output_64(arg1);
write_output_64(arg2);
}
// If we have 64 bits arguments then write them in Little-endian.
write_output((uint32)(arg1 & 0xFFFFFFFF));
write_output((uint32)(arg1 >> 32));
write_output((uint32)(arg2 & 0xFFFFFFFF));
write_output((uint32)(arg2 >> 32));
}

bool kcov_comparison_t::ignore() const
Expand Down
6 changes: 4 additions & 2 deletions executor/executor_linux.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,10 @@ static void cover_reset(cover_t* cov)

static void cover_collect(cover_t* cov)
{
// Note: this assumes little-endian kernel.
cov->size = *(uint32*)cov->data;
if (is_kernel_64_bit)
cov->size = *(uint64*)cov->data;
else
cov->size = *(uint32*)cov->data;
}

static bool cover_check(uint32 pc)
Expand Down
4 changes: 2 additions & 2 deletions executor/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
static int test_copyin()
{
static uint16 buf[3];
STORE_BY_BITMASK(uint16, , &buf[1], 0x1234, 0, 16);
STORE_BY_BITMASK(uint16, htole16, &buf[1], 0x1234, 0, 16);
unsigned char x[sizeof(buf)];
memcpy(x, buf, sizeof(x));
if (x[0] != 0 || x[1] != 0 ||
Expand All @@ -18,7 +18,7 @@ static int test_copyin()
x[0], x[1], x[2], x[3], x[4], x[5]);
return 1;
}
STORE_BY_BITMASK(uint16, , &buf[1], 0x555a, 5, 4);
STORE_BY_BITMASK(uint16, htole16, &buf[1], 0x555a, 5, 4);
memcpy(x, buf, sizeof(x));
if (x[0] != 0 || x[1] != 0 ||
x[2] != 0x54 || x[3] != 0x13 ||
Expand Down
5 changes: 2 additions & 3 deletions pkg/ipc/ipc.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package ipc

import (
"encoding/binary"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -439,7 +438,7 @@ func readUint32(outp *[]byte) (uint32, bool) {
if len(out) < 4 {
return 0, false
}
v := binary.LittleEndian.Uint32(out)
v := prog.HostEndian.Uint32(out)
*outp = out[4:]
return v, true
}
Expand All @@ -449,7 +448,7 @@ func readUint64(outp *[]byte) (uint64, bool) {
if len(out) < 8 {
return 0, false
}
v := binary.LittleEndian.Uint64(out)
v := prog.HostEndian.Uint64(out)
*outp = out[8:]
return v, true
}
Expand Down
10 changes: 10 additions & 0 deletions prog/big_endian.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2020 syzkaller project authors. All rights reserved.
// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.

// +build s390x

package prog

import "encoding/binary"

var HostEndian = binary.BigEndian
5 changes: 1 addition & 4 deletions prog/decodeexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,7 @@ func (dec *execDecoder) read() uint64 {
if dec.err != nil {
return 0
}
var v uint64
for i := 0; i < 8; i++ {
v |= uint64(dec.data[i]) << uint(i*8)
}
v := HostEndian.Uint64(dec.data)
dec.data = dec.data[8:]
return v
}
Expand Down
13 changes: 5 additions & 8 deletions prog/encodingexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package prog

import (
"bytes"
"encoding/binary"
"fmt"
"sort"
)
Expand Down Expand Up @@ -221,14 +223,9 @@ func (w *execContext) write(v uint64) {
w.eof = true
return
}
w.buf[0] = byte(v >> 0)
w.buf[1] = byte(v >> 8)
w.buf[2] = byte(v >> 16)
w.buf[3] = byte(v >> 24)
w.buf[4] = byte(v >> 32)
w.buf[5] = byte(v >> 40)
w.buf[6] = byte(v >> 48)
w.buf[7] = byte(v >> 56)
buf := new(bytes.Buffer)
binary.Write(buf, HostEndian, v)
copy(w.buf, buf.Bytes())
w.buf = w.buf[8:]
}

Expand Down
23 changes: 17 additions & 6 deletions prog/encodingexec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ func TestSerializeForExec(t *testing.T) {
}
return uint64(c.ID)
}
letoh64 := func(v uint64) uint64 {
buf := make([]byte, 8)
buf[0] = byte(v >> 0)
buf[1] = byte(v >> 8)
buf[2] = byte(v >> 16)
buf[3] = byte(v >> 24)
buf[4] = byte(v >> 32)
buf[5] = byte(v >> 40)
buf[6] = byte(v >> 48)
buf[7] = byte(v >> 56)
return HostEndian.Uint64(buf)
}
tests := []struct {
prog string
serialized []uint64
Expand Down Expand Up @@ -204,7 +216,7 @@ func TestSerializeForExec(t *testing.T) {
"test$array1(&(0x7f0000000000)={0x42, \"0102030405\"})",
[]uint64{
execInstrCopyin, dataOffset + 0, execArgConst, 1, 0x42,
execInstrCopyin, dataOffset + 1, execArgData, 5, 0x0504030201,
execInstrCopyin, dataOffset + 1, execArgData, 5, letoh64(0x0504030201),
callID("test$array1"), ExecNoCopyout, 1, execArgConst, ptrSize, dataOffset,
execInstrEOF,
},
Expand All @@ -214,7 +226,7 @@ func TestSerializeForExec(t *testing.T) {
"test$array2(&(0x7f0000000000)={0x42, \"aaaaaaaabbbbbbbbccccccccdddddddd\", 0x43})",
[]uint64{
execInstrCopyin, dataOffset + 0, execArgConst, 2, 0x42,
execInstrCopyin, dataOffset + 2, execArgData, 16, 0xbbbbbbbbaaaaaaaa, 0xddddddddcccccccc,
execInstrCopyin, dataOffset + 2, execArgData, 16, letoh64(0xbbbbbbbbaaaaaaaa), letoh64(0xddddddddcccccccc),
execInstrCopyin, dataOffset + 18, execArgConst, 2, 0x43,
callID("test$array2"), ExecNoCopyout, 1, execArgConst, ptrSize, dataOffset,
execInstrEOF,
Expand Down Expand Up @@ -436,8 +448,7 @@ func TestSerializeForExec(t *testing.T) {
execInstrCopyin, dataOffset + 2, execArgConst, 4 | 1<<8, 0x1,
execInstrCopyin, dataOffset + 6, execArgConst, 4 | 1<<8, 0x2,
execInstrCopyin, dataOffset + 10, execArgConst, 2, 0x0,
execInstrCopyin, dataOffset + 12, execArgData, 1, 0xab,

execInstrCopyin, dataOffset + 12, execArgData, 1, letoh64(0xab),
execInstrCopyin, dataOffset + 10, execArgCsum, 2, ExecArgCsumInet, 5,
ExecArgCsumChunkData, dataOffset + 2, 4,
ExecArgCsumChunkData, dataOffset + 6, 4,
Expand Down Expand Up @@ -466,11 +477,11 @@ func TestSerializeForExec(t *testing.T) {
t.Fatalf("failed to serialize: %v", err)
}
w := new(bytes.Buffer)
binary.Write(w, binary.LittleEndian, test.serialized)
binary.Write(w, HostEndian, test.serialized)
data := buf[:n]
if !bytes.Equal(data, w.Bytes()) {
got := make([]uint64, len(data)/8)
binary.Read(bytes.NewReader(data), binary.LittleEndian, &got)
binary.Read(bytes.NewReader(data), HostEndian, &got)
t.Logf("want: %v", test.serialized)
t.Logf("got: %v", got)
t.Fatalf("mismatch")
Expand Down
10 changes: 10 additions & 0 deletions prog/little_endian.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2020 syzkaller project authors. All rights reserved.
// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.

// +build amd64 386 arm64 arm mips64le ppc64le

package prog

import "encoding/binary"

var HostEndian = binary.LittleEndian