From d082a7473af1900321af728de4ae7f4557c4fbf6 Mon Sep 17 00:00:00 2001 From: Achilleas Anagnostopoulos Date: Thu, 22 Mar 2018 08:01:22 +0000 Subject: [PATCH 1/2] vmm: handle mapping of kernel ELF sections that are not page aligned The code responsible for mapping the kernel sections worked under the assumption that the linker would align all sections on a page boundary. To figure out the page extents for a particular section, the implementation would round the section VMA down to the nearest page and add to that the section length rounded up to the nearest page size. However, some sections (e.g. noptrbss) use 32 byte alignment. Depending on the section length, the original implementation could in some cases skip mapping of the last page in the section which would cause a page fault when its contents were accessed. The fix for this issue is quite simple: calculate the end page by rounding up (section start addr + section length) to the next page. --- src/gopheros/kernel/mem/vmm/vmm.go | 11 +++++++---- src/gopheros/kernel/mem/vmm/vmm_test.go | 20 ++++++++++++-------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/gopheros/kernel/mem/vmm/vmm.go b/src/gopheros/kernel/mem/vmm/vmm.go index 1e8fa98..16bc74a 100644 --- a/src/gopheros/kernel/mem/vmm/vmm.go +++ b/src/gopheros/kernel/mem/vmm/vmm.go @@ -181,7 +181,6 @@ func setupPDTForKernel(kernelPageOffset uintptr) *kernel.Error { // Query the ELF sections of the kernel image and establish mappings // for each one using the appropriate flags - pageSizeMinus1 := uint64(mem.PageSize - 1) var visitor = func(_ string, secFlags multiboot.ElfSectionFlag, secAddress uintptr, secSize uint64) { // Bail out if we have encountered an error; also ignore sections // not using the kernel's VMA @@ -199,11 +198,15 @@ func setupPDTForKernel(kernelPageOffset uintptr) *kernel.Error { flags |= FlagRW } - // We assume that all sections are page-aligned by the linker script + // Map the start and end VMA addresses for the section contents + // into a start and end (inclusive) page number. To figure out + // the physical start frame we just need to subtract the + // kernel's VMA offset from the virtual address and round that + // down to the nearest frame number. curPage := PageFromAddress(secAddress) + lastPage := PageFromAddress(secAddress + uintptr(secSize-1)) curFrame := pmm.Frame((secAddress - kernelPageOffset) >> mem.PageShift) - endFrame := curFrame + pmm.Frame(((secSize+pageSizeMinus1) & ^pageSizeMinus1)>>mem.PageShift) - for ; curFrame < endFrame; curFrame, curPage = curFrame+1, curPage+1 { + for ; curPage <= lastPage; curFrame, curPage = curFrame+1, curPage+1 { if err = pdt.Map(curPage, curFrame, flags); err != nil { return } diff --git a/src/gopheros/kernel/mem/vmm/vmm_test.go b/src/gopheros/kernel/mem/vmm/vmm_test.go index c9b8fa3..fd768fd 100644 --- a/src/gopheros/kernel/mem/vmm/vmm_test.go +++ b/src/gopheros/kernel/mem/vmm/vmm_test.go @@ -330,10 +330,14 @@ func TestSetupPDTForKernel(t *testing.T) { translateFn = func(_ uintptr) (uintptr, *kernel.Error) { return 0xbadf00d000, nil } mapTemporaryFn = func(f pmm.Frame) (Page, *kernel.Error) { return Page(f), nil } visitElfSectionsFn = func(v multiboot.ElfSectionVisitor) { - v(".debug", 0, 0, uint64(mem.PageSize>>1)) // address < VMA; should be ignored - v(".text", multiboot.ElfSectionExecutable, 0xbadc0ffee, uint64(mem.PageSize>>1)) - v(".data", multiboot.ElfSectionWritable, 0xbadc0ffee, uint64(mem.PageSize)) - v(".rodata", 0, 0xbadc0ffee, uint64(mem.PageSize<<1)) + // address < VMA; should be ignored + v(".debug", 0, 0, uint64(mem.PageSize>>1)) + // section uses 32-byte alignment instead of page alignment and has a size + // equal to 1 page. Due to rounding, we need to actually map 2 pages. + v(".text", multiboot.ElfSectionExecutable, 0x10032, uint64(mem.PageSize)) + v(".data", multiboot.ElfSectionWritable, 0x2000, uint64(mem.PageSize)) + // section is page-aligned and occupies exactly 2 pages + v(".rodata", 0, 0x3000, uint64(mem.PageSize<<1)) } mapCount := 0 mapFn = func(page Page, frame pmm.Frame, flags PageTableEntryFlag) *kernel.Error { @@ -342,11 +346,11 @@ func TestSetupPDTForKernel(t *testing.T) { var expFlags PageTableEntryFlag switch mapCount { - case 0: + case 0, 1: expFlags = FlagPresent - case 1: + case 2: expFlags = FlagPresent | FlagNoExecute | FlagRW - case 2, 3: + case 3, 4: expFlags = FlagPresent | FlagNoExecute } @@ -361,7 +365,7 @@ func TestSetupPDTForKernel(t *testing.T) { t.Fatal(err) } - if exp := 4; mapCount != exp { + if exp := 5; mapCount != exp { t.Errorf("expected Map to be called %d times; got %d", exp, mapCount) } }) From 76f7869f57f63b4a9045125f27ac749149f3b740 Mon Sep 17 00:00:00 2001 From: Achilleas Anagnostopoulos Date: Thu, 22 Mar 2018 08:09:58 +0000 Subject: [PATCH 2/2] vmm: fix some lint warnings (error not checked) --- src/gopheros/kernel/mem/vmm/pdt.go | 2 +- src/gopheros/kernel/mem/vmm/vmm.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gopheros/kernel/mem/vmm/pdt.go b/src/gopheros/kernel/mem/vmm/pdt.go index 012ceba..fbbac87 100644 --- a/src/gopheros/kernel/mem/vmm/pdt.go +++ b/src/gopheros/kernel/mem/vmm/pdt.go @@ -63,7 +63,7 @@ func (pdt *PageDirectoryTable) Init(pdtFrame pmm.Frame) *kernel.Error { lastPdtEntry.SetFrame(pdtFrame) // Remove temporary mapping - unmapFn(pdtPage) + _ = unmapFn(pdtPage) return nil } diff --git a/src/gopheros/kernel/mem/vmm/vmm.go b/src/gopheros/kernel/mem/vmm/vmm.go index 16bc74a..8fcc6cd 100644 --- a/src/gopheros/kernel/mem/vmm/vmm.go +++ b/src/gopheros/kernel/mem/vmm/vmm.go @@ -69,7 +69,7 @@ func pageFaultHandler(errorCode uint64, frame *irq.Frame, regs *irq.Regs) { } else { // Copy page contents, mark as RW and remove CoW flag mem.Memcopy(faultPage.Address(), tmpPage.Address(), mem.PageSize) - unmapFn(tmpPage) + _ = unmapFn(tmpPage) // Update mapping to point to the new frame, flag it as RW and // remove the CoW flag @@ -139,7 +139,7 @@ func reserveZeroedFrame() *kernel.Error { return err } mem.Memset(tempPage.Address(), 0, mem.PageSize) - unmapFn(tempPage) + _ = unmapFn(tempPage) // From this point on, ReservedZeroedFrame cannot be mapped with a RW flag protectReservedZeroedPage = true