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

runtime: use PAGE_NO­ACCESS in sysReseve #33327

Open
alexbrainman opened this issue Jul 28, 2019 · 1 comment
Open

runtime: use PAGE_NO­ACCESS in sysReseve #33327

alexbrainman opened this issue Jul 28, 2019 · 1 comment

Comments

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jul 28, 2019

Windows version of runtime.sysReseve calls VirtualAlloc with MEM_RESERVE and PAGE_READWRITE, but it should use PAGE_NO­ACCESS when using MEM_RESERVE.

According to https://devblogs.microsoft.com/oldnewthing/20171227-00/?p=97656

Why do you have to pass a valid value even if the system doesn’t use it?

This is an artifact of how the front-end parameter validation is done. The Virtual­Alloc function does parameter validation by checking each parameter individually to confirm that the value is among the valid values.

and in particular

Bonus chatter: You would think that the flProtect would not be used when reserving address space with MEM_RESERVE, but you’d be wrong. If reserving regular address space, then the protection should be PAGE_NO­ACCESS.

Someone would have to try and see what effect this change have on memory used by Go executable before making this change. Maybe by using vmmap or rammap tools.

I am creating this issue before I forget.

/cc @aclements and @zx2c4 because you might be interested.

Alex

@alexbrainman

This comment has been minimized.

Copy link
Member Author

@alexbrainman alexbrainman commented Jul 28, 2019

I tried changing

diff --git a/src/runtime/mem_windows.go b/src/runtime/mem_windows.go
index 165062e..3ccb690 100644
--- a/src/runtime/mem_windows.go
+++ b/src/runtime/mem_windows.go
@@ -115,13 +115,13 @@ func sysReserve(v unsafe.Pointer, n uintptr) unsafe.Pointer {
 	// v is just a hint.
 	// First try at v.
 	// This will fail if any of [v, v+n) is already reserved.
-	v = unsafe.Pointer(stdcall4(_VirtualAlloc, uintptr(v), n, _MEM_RESERVE, _PAGE_READWRITE))
+	v = unsafe.Pointer(stdcall4(_VirtualAlloc, uintptr(v), n, _MEM_RESERVE, _PAGE_NOACCESS))
 	if v != nil {
 		return v
 	}
 
 	// Next let the kernel choose the address.
-	return unsafe.Pointer(stdcall4(_VirtualAlloc, 0, n, _MEM_RESERVE, _PAGE_READWRITE))
+	return unsafe.Pointer(stdcall4(_VirtualAlloc, 0, n, _MEM_RESERVE, _PAGE_NOACCESS))
 }
 
 func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) {

and I run this program

package main

import (
	"time"
)

func main() {
	time.Sleep(time.Hour)
}

and I used vmmap to compare memory map before and after the change. And I don't see any differences.

image

I use up to date version of Windows 10.

Alex

@ALTree ALTree added this to the Unplanned milestone Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.