Skip to content
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
2 changes: 1 addition & 1 deletion executor/common_kvm_amd64.h
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,7 @@ static void install_syzos_code(void* host_mem, size_t mem_size)
{
size_t size = (char*)&__stop_guest - (char*)&__start_guest;
if (size > mem_size)
fail("SyzOS size exceeds guest memory");
fail("SYZOS size exceeds guest memory");
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for fixing the naming in the existing code, but please put it into a separate commit.

memcpy(host_mem, &__start_guest, size);
}

Expand Down
43 changes: 7 additions & 36 deletions executor/common_kvm_amd64_syzos.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@

// This file provides guest code running inside the AMD64 KVM.

#include "common_kvm_syzos.h"
#include "kvm.h"
#include <linux/kvm.h>
#include <stdbool.h>

#include "common_kvm_syzos.h"
#include "kvm.h"

// There are no particular rules to assign numbers here, but changing them will
// result in losing some existing reproducers. Therefore, we try to leave spaces
// between unrelated IDs.
Expand Down Expand Up @@ -43,16 +44,6 @@ typedef enum {
SYZOS_API_STOP, // Must be the last one
} syzos_api_id;

struct api_call_header {
Copy link
Member

Choose a reason for hiding this comment

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

Please make this a separate commit that only touches x86/ARM and brings no functional change.

uint64 call;
uint64 size;
};

struct api_call_uexit {
struct api_call_header header;
uint64 exit_code;
};

struct api_call_code {
struct api_call_header header;
uint8 insns[];
Expand All @@ -70,26 +61,6 @@ struct api_call_cpuid {
uint32 ecx;
};

struct api_call_1 {
struct api_call_header header;
uint64 arg;
};

struct api_call_2 {
struct api_call_header header;
uint64 args[2];
};

struct api_call_3 {
struct api_call_header header;
uint64 args[3];
};

struct api_call_5 {
struct api_call_header header;
uint64 args[5];
};

// This struct must match the push/pop order in nested_vm_exit_handler_intel_asm().
struct l2_guest_regs {
uint64 rax, rbx, rcx, rdx, rsi, rdi, rbp;
Expand Down Expand Up @@ -165,8 +136,8 @@ __attribute__((naked)) GUEST_CODE static void uexit_irq_handler()
// TODO(glider): executor/style_test.go insists that single-line compound statements should not
// be used e.g. in the following case:
// if (call == SYZOS_API_UEXIT) {
// struct api_call_uexit* ucmd = (struct api_call_uexit*)cmd;
// guest_uexit(ucmd->exit_code);
// struct api_call_1* ccmd = (struct api_call_1*)cmd;
// guest_uexit(ccmd->arg);
// } else if (call == SYZOS_API_WR_CRN) {
// guest_handle_wr_crn((struct api_call_2*)cmd); // Style check fails here
// }
Expand All @@ -188,8 +159,8 @@ guest_main(uint64 size, uint64 cpu)
volatile uint64 call = cmd->call;
if (call == SYZOS_API_UEXIT) {
// Issue a user exit.
struct api_call_uexit* ucmd = (struct api_call_uexit*)cmd;
guest_uexit(ucmd->exit_code);
struct api_call_1* ccmd = (struct api_call_1*)cmd;
guest_uexit(ccmd->arg);
} else if (call == SYZOS_API_CODE) {
// Execute an instruction blob.
struct api_call_code* ccmd = (struct api_call_code*)cmd;
Expand Down
8 changes: 4 additions & 4 deletions executor/common_kvm_arm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ static void vm_set_user_memory_region(int vmfd, uint32 slot, uint32 flags, uint6
#define ADRP_OPCODE 0x90000000
#define ADRP_OPCODE_MASK 0x9f000000

// Code loading SyzOS into guest memory does not handle data relocations (see
// https://github.com/google/syzkaller/issues/5565), so SyzOS will crash soon after encountering an
// Code loading SYZOS into guest memory does not handle data relocations (see
Copy link
Member

Choose a reason for hiding this comment

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

Same here - please group all s/SyzOS/SYZOS into a separate commit.

// https://github.com/google/syzkaller/issues/5565), so SYZOS will crash soon after encountering an
// ADRP instruction. Detect these instructions to catch regressions early.
// The most common reason for using data relocaions is accessing global variables and constants.
// Sometimes the compiler may choose to emit a read-only constant to zero-initialize a structure
Expand All @@ -81,15 +81,15 @@ static void validate_guest_code(void* mem, size_t size)
uint32* insns = (uint32*)mem;
for (size_t i = 0; i < size / 4; i++) {
if ((insns[i] & ADRP_OPCODE_MASK) == ADRP_OPCODE)
fail("ADRP instruction detected in SyzOS, exiting");
fail("ADRP instruction detected in SYZOS, exiting");
}
}

static void install_syzos_code(void* host_mem, size_t mem_size)
{
size_t size = (char*)&__stop_guest - (char*)&__start_guest;
if (size > mem_size)
fail("SyzOS size exceeds guest memory");
fail("SYZOS size exceeds guest memory");
memcpy(host_mem, &__start_guest, size);
validate_guest_code(host_mem, size);
}
Expand Down
36 changes: 6 additions & 30 deletions executor/common_kvm_arm64_syzos.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@

// This file provides guest code running inside the ARM64 KVM.

#include "common_kvm_syzos.h"
#include "kvm.h"
#include <linux/kvm.h>
#include <stdbool.h>

#include "common_kvm_syzos.h"
#include "kvm.h"

// Compilers will eagerly try to transform the switch statement in guest_main()
// into a jump table, unless the cases are sparse enough.
// We use prime numbers multiplied by 10 to prevent this behavior.
Expand All @@ -31,31 +32,6 @@ typedef enum {
SYZOS_API_STOP, // Must be the last one
} syzos_api_id;

struct api_call_header {
uint64 call;
uint64 size;
};

struct api_call_uexit {
struct api_call_header header;
uint64 exit_code;
};

struct api_call_1 {
struct api_call_header header;
uint64 arg;
};

struct api_call_2 {
struct api_call_header header;
uint64 args[2];
};

struct api_call_3 {
struct api_call_header header;
uint64 args[3];
};

struct api_call_code {
struct api_call_header header;
uint32 insns[];
Expand Down Expand Up @@ -127,8 +103,8 @@ guest_main(uint64 size, uint64 cpu)
return;
switch (cmd->call) {
case SYZOS_API_UEXIT: {
struct api_call_uexit* ucmd = (struct api_call_uexit*)cmd;
guest_uexit(ucmd->exit_code);
struct api_call_1* ccmd = (struct api_call_1*)cmd;
guest_uexit(ccmd->arg);
break;
}
case SYZOS_API_CODE: {
Expand Down Expand Up @@ -1219,7 +1195,7 @@ GUEST_CODE static void its_send_sync_cmd(uint64 cmdq_base, uint32 vcpu_id)
}

// This function is carefully written in a way that prevents jump table emission.
// SyzOS cannot reference global constants, and compilers are very eager to generate a jump table
// SYZOS cannot reference global constants, and compilers are very eager to generate a jump table
// for a switch over GITS commands.
// To work around that, we replace the switch statement with a series of if statements.
// In addition, cmd->type is stored in a volatile variable, so that it is read on each if statement,
Expand Down
Loading
Loading