diff --git a/pkg/winacl/api/acl.go b/pkg/winacl/api/acl.go index c5063a4a..9776550a 100644 --- a/pkg/winacl/api/acl.go +++ b/pkg/winacl/api/acl.go @@ -4,7 +4,6 @@ package api import ( - "runtime" "syscall" "unsafe" @@ -92,24 +91,6 @@ func SetEntriesInAcl(entries []ExplicitAccess, oldAcl windows.Handle, newAcl *wi return nil } - // Pin every pointer the kernel will dereference *transitively* through - // the entries slice. The unsafe.Pointer->uintptr pinning rule inside - // syscall.SyscallN only pins what is passed directly as a uintptr arg - // (here, &entries[0]). The kernel also reads through - // entries[i].Trustee.Name -- a *uint16 that, depending on TrusteeForm, - // is either a UTF-16 string or a SID pointer (cast via - // (*uint16)(unsafe.Pointer(sid))). Those targets are NOT covered by - // the inline pinning rule, and the typed-as-*uint16 cast obscures the - // true allocation from escape analysis, which has been the source of - // delayed memory-corruption crashes on long-running Windows scanners. - var pinner runtime.Pinner - defer pinner.Unpin() - for i := range entries { - if entries[i].Trustee.Name != nil { - pinner.Pin(entries[i].Trustee.Name) - } - } - ret, _, _ := syscall.SyscallN( procSetEntriesInAclW.Addr(), uintptr(len(entries)), diff --git a/pkg/winacl/apply.go b/pkg/winacl/apply.go index 1294c7ec..2a07c302 100644 --- a/pkg/winacl/apply.go +++ b/pkg/winacl/apply.go @@ -3,6 +3,8 @@ package winacl import ( + "runtime" + "github.com/runZeroInc/go-rod/pkg/winacl/api" "golang.org/x/sys/windows" ) @@ -26,6 +28,28 @@ func Apply(name string, replace, inherit bool, entries ...api.ExplicitAccess) er ) defer windows.LocalFree(secDesc) } + + // Pin every pointer the kernel will dereference *transitively* through + // the entries slice. The unsafe.Pointer->uintptr pinning rule inside + // syscall.SyscallN only pins what is passed directly as a uintptr arg + // (here, &entries[0]). The kernel also reads through + // entries[i].Trustee.Name -- a *uint16 that, depending on TrusteeForm, + // is either a UTF-16 string or a SID pointer (cast via + // (*uint16)(unsafe.Pointer(sid))). Those targets are NOT covered by + // the inline pinning rule, and the typed-as-*uint16 cast obscures the + // true allocation from escape analysis, which has been the source of + // delayed memory-corruption crashes on long-running Windows scanners. + var pinner runtime.Pinner + defer pinner.Unpin() + if len(entries) > 0 { + pinner.Pin(&entries[0]) + } + for i := range entries { + if entries[i].Trustee.Name != nil { + pinner.Pin(entries[i].Trustee.Name) + } + } + var acl windows.Handle if err := api.SetEntriesInAcl( entries, @@ -35,6 +59,7 @@ func Apply(name string, replace, inherit bool, entries ...api.ExplicitAccess) er return err } defer windows.LocalFree(acl) + var secInfo uint32 if !inherit { secInfo = api.PROTECTED_DACL_SECURITY_INFORMATION diff --git a/pkg/winaclx/helpers.go b/pkg/winaclx/helpers.go index 4ad3b9cb..3856c414 100644 --- a/pkg/winaclx/helpers.go +++ b/pkg/winaclx/helpers.go @@ -235,6 +235,24 @@ func osWindowsApplyACL(name string, replace, inherit bool, entries ...ExplicitAc ) defer windows.LocalFree(secDesc) } + + // Pin every pointer the kernel will dereference *transitively* through + // entries[i].Trustee.Name. The unsafe.Pointer->uintptr pinning rule in + // syscall.SyscallN only pins direct arguments (&entries[0]); pointers + // nested inside that struct -- here, the SID or UTF-16 buffer cast to + // *uint16 -- are NOT covered, and the cast through a smaller type + // obscures the true allocation from escape analysis. + var pinner runtime.Pinner + defer pinner.Unpin() + if len(entries) > 0 { + pinner.Pin(&entries[0]) + } + for i := range entries { + if entries[i].Trustee.Name != nil { + pinner.Pin(entries[i].Trustee.Name) + } + } + var acl windows.Handle if err := SetEntriesInAcl( entries, @@ -244,6 +262,7 @@ func osWindowsApplyACL(name string, replace, inherit bool, entries ...ExplicitAc return err } defer windows.LocalFree(acl) + var secInfo uint32 if !inherit { secInfo = PROTECTED_DACL_SECURITY_INFORMATION @@ -299,20 +318,6 @@ func SetEntriesInAcl(entries []ExplicitAccess, oldAcl windows.Handle, newAcl *wi return nil } - // Pin every pointer the kernel will dereference *transitively* through - // entries[i].Trustee.Name. The unsafe.Pointer->uintptr pinning rule in - // syscall.SyscallN only pins direct arguments (&entries[0]); pointers - // nested inside that struct -- here, the SID or UTF-16 buffer cast to - // *uint16 -- are NOT covered, and the cast through a smaller type - // obscures the true allocation from escape analysis. - var pinner runtime.Pinner - defer pinner.Unpin() - for i := range entries { - if entries[i].Trustee.Name != nil { - pinner.Pin(entries[i].Trustee.Name) - } - } - ret, _, _ := syscall.SyscallN( procSetEntriesInAclW.Addr(), uintptr(len(entries)),