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
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@alexbrainman
Copy link
Member

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
Copy link
Member Author

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 the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jul 29, 2019
@ALTree ALTree added this to the Unplanned milestone Jul 29, 2019
@ALTree ALTree added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants