Skip to content

Commit

Permalink
boot/paging: add work-around for 32-bit codegen bug in Zig
Browse files Browse the repository at this point in the history
Turns out there is a codegen bug in Zig 0.9.1 when
generating 32-bit code that operate on 64-bit arrays.

In particular, `foo[index] = bar`, where `foo` is an
`[512]u64` array, causes the high 32-bit of `bar` to be
discarded.

The assembly code actually attempts to set the high 32-bit
of bar, but uses the incorrect offset, and thus overwrites
the value with the low 32-bit part of bar.

This issue was identified as the NX bit was _not_ set in
the p2_table, even though the Zig code contained a
`page_table_entry.no_execute = true` statement.

Gotta love compiler bugs that makes mapped pages w+x :)

Tribute to Ken on Trusting Trust <3

Prior to this fix, `info tlb` gave:

	(qemu) info tlb
	// kernel code
	0000000000000000: 0000000000000000 --PDA--UW
	// kernel data:
	0000000000200000: 0000000000200000 --PDA--UW
	...
	0000000003e00000: 0000000003e00000 --P----UW
	// kernel stack (guard)
	0000000004000000: 0000000004000000 --P----U-
	// kernel stack
	0000000004200000: 0000000004200000 --P----UW
	0000000004400000: 0000000004400000 --PDA--UW
	// kernel stack (guard)
	0000000004600000: 0000000004600000 --P----U-
	// frame buffer:
	0000000004800000: 00000000fd000000 --PDA--UW
	0000000004a00000: 00000000fd200000 --PDA--UW
	0000000004c00000: 00000000fd400000 --PDA--UW

After this commit, `info tlb` gives:

	(qemu) info tlb
	// kernel code
	0000000000000000: 0000000000000000 --PDA--UW
	// kernel data:
	0000000000200000: 0000000000200000 X-PDA--UW
	...
	0000000003e00000: 0000000003e00000 X-P----UW
	// kernel stack (guard)
	0000000004000000: 0000000004000000 X-P----U-
	// kernel stack
	0000000004200000: 0000000004200000 X-P----UW
	0000000004400000: 0000000004400000 X-PDA--UW
	// kernel stack (guard)
	0000000004600000: 0000000004600000 X-P----U-
	// frame buffer:
	0000000004800000: 00000000fd000000 X-PDA--UW
	0000000004a00000: 00000000fd200000 X-PDA--UW
	0000000004c00000: 00000000fd400000 X-PDA--UW

For the morbidly curious, this is the incorrect assembly as
generated by Zig 0.9.1 32-bit codegen:

	0x1006a5 <map_kernel_stack+341> mov    ecx, DWORD PTR [rbp-0x8]        ; 0x4200087
	0x1006a8 <map_kernel_stack+344> mov    edx, DWORD PTR [rbp-0x4]        ; 0x80000000
	0x1006ab <map_kernel_stack+347> mov    DWORD PTR [rax*8+0x112000], edx ; BUG! writes high 32-bit part of page_table_entry (including NX bit) to incorrect offset.
	0x1006b2 <map_kernel_stack+354> mov    DWORD PTR [rax*8+0x112000], ecx ; overwrites previous write with low 32-bit part of page_table_entry (overwrites NX bit).
  • Loading branch information
mewmew committed May 26, 2022
1 parent ab5cc59 commit 1f0b42f
Showing 1 changed file with 16 additions and 4 deletions.
20 changes: 16 additions & 4 deletions src/boot/paging_32.zig
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ export fn map_kernel_code_segment() void {
page_table_entry.setAddr(addr);
// Set page table entry.
const p2_index = page_num + p2_index_offset;
p2_table[p2_index] = page_table_entry.entry;
// TODO: uncomment when Zig 32-bit codegen bug is fixed; use work-around for now.
//p2_table[p2_index] = page_table_entry.entry;
var p: *u64 = &p2_table[p2_index];
p.* = page_table_entry.entry;
}
}

Expand All @@ -87,7 +90,10 @@ export fn map_kernel_data_segment() void {
page_table_entry.setAddr(addr);
// Set page table entry.
const p2_index = page_num + p2_index_offset;
p2_table[p2_index] = page_table_entry.entry;
// TODO: uncomment when Zig 32-bit codegen bug is fixed; use work-around for now.
//p2_table[p2_index] = page_table_entry.entry;
var p: *u64 = &p2_table[p2_index];
p.* = page_table_entry.entry;
}
}

Expand Down Expand Up @@ -118,7 +124,10 @@ export fn map_kernel_stack() void {
page_table_entry.setAddr(addr);
// Set page table entry.
const p2_index = page_num + p2_index_offset;
p2_table[p2_index] = page_table_entry.entry;
// TODO: uncomment when Zig 32-bit codegen bug is fixed; use work-around for now.
//p2_table[p2_index] = page_table_entry.entry;
var p: *u64 = &p2_table[p2_index];
p.* = page_table_entry.entry;
}
}

Expand Down Expand Up @@ -148,7 +157,10 @@ export fn map_frame_buffer() void {
page_table_entry.setAddr(addr);
// Set page table entry.
const p2_index = page_num + p2_index_offset;
p2_table[p2_index] = page_table_entry.entry;
// TODO: uncomment when Zig 32-bit codegen bug is fixed; use work-around for now.
//p2_table[p2_index] = page_table_entry.entry;
var p: *u64 = &p2_table[p2_index];
p.* = page_table_entry.entry;
}
}

Expand Down

0 comments on commit 1f0b42f

Please sign in to comment.