Memory corruption and possible memory leak due to ListLoggedInUsers()

[yusufozturk]

Due to misuse of unsafe pointers, after a while go process crashes due to memory corruption.

Here is the output when memory corruption happens:
fatal error: unexpected signal during runtime execution
[signal 0xc0000005 code=0x1 addr=0xc0008cb910 pc=0xa4e77a]

After solving the issue, I will try to send a pull request.

[si458]

i think i have the same issue,

fatal error: unexpected signal during runtime execution
[signal 0xc0000005 code=0x1 addr=0xc000407920 pc=0xe5da7a]

im simply calling the ListLoggedInUsers every 5 seconds to check if the is any users logged in
and watching the task manager shows the memory increasing every single time

did you have any luck @yusufozturk with fixing this?

[yusufozturk]

@si458 Yes I did. I will share it here. What I did, solved the memory corruption but memory leak is still happening. I fixed that by using WMI query to check logged users as a cache and if there is a change, then I use this method to bring user information. A bit dirty but works :)

[si458]

@yusufozturk brilliant! looking forward to request/share!

[yusufozturk]

@si458 sorry for the delay. here is the code.
session_windows.txt

[si458]

@yusufozturk code looks go, thanks!
is the a reason you removed the LocalAdmin from what gets returned? and added State as Active?

[yusufozturk]

Aah sorry, wait, it was the wrong branch :) I'm sending the latest code, just made for our needs. We fill the state already so we can use it somewhere else. Also even the old code was keep crashing the program, so we tried to remove many parts that we don't need.

session_windows.txt

If you need to use caching just like us, I would suggest following wmi query:

// Get Currently Logged On Users	
loggedOnUsers, err := wmi.Query("ROOT\\CIMV2", "Select Antecedent From Win32_LoggedOnUser", device, mh, ch)

We check if there is any change via WMI, and if there is a change then we get the logged on list.

I hope this would help you too. I would say this one works very long already. There are no memory corruption anymore but there is still some memory leak, so we fixed that by using caching.

[yusufozturk]

@si458 for wmi, we use a custom package of stackexchange/wmi. I believe this function makes the memory leak:

func (us *LSA_UNICODE_STRING) String() string {
	return syscall.UTF16ToString((*[4096]uint16)(unsafe.Pointer(us.Buffer))[:us.Length]) //nolint:unsafeptr goland:noinspection GoVetUnsafePointer
}

but we weren't able to fix it, so we added caching.

[iamacarpet]

If you think it's related to the LSA_UNICODE_STRING stuff, that memory is allocated by the Windows kernel (via whatever C function it is running inside the DLL to return the results)...

Perhaps this call is failing?

sessLsaFreeReturnBuffer

Which should be telling Windows it can free the memory (as since it allocated it, it needs to do manual memory management to free it, C style).

Allocated:
https://github.com/iamacarpet/go-win64api/blob/master/sessions.go#L65
https://github.com/iamacarpet/go-win64api/blob/master/sessions.go#L75

Freed:
https://github.com/iamacarpet/go-win64api/blob/master/sessions.go#L69
https://github.com/iamacarpet/go-win64api/blob/master/sessions.go#L118

As for the memory corruption, we've got quite a large deployment checking every 30 seconds with this call and have never seen it in our bug reporting, but perhaps we aren't checking frequently enough or something.

I was under the impression Go was copying the strings to it's own memory locations when we convert them to native types, but perhaps I'm wrong and underneath it is still referencing the kernel allocated memory regions?

It would make sense them that if the sessLsaFreeReturnBuffer call occasionally works, when you try to access the string it is seen as memory corruption, as we're accessing memory that has been freed...

I have noticed an inconsistency between the two calls to sessLsaFreeReturnBuffer: one is passing the pointer with a prefix of "&" and the other isn't, which AFAIK makes quite a huge difference to what is actually presented: maybe some trial and error around that might find the bug?

[davidnewhall]

Just putting in my useless comment. My users also experience this crash with an app that calls ListLoggedInUsers on a timer. Let me know if I can provide any more helpful details.

goroutine 48867 [running]:
runtime.throw({0xc02af5?, 0x2492e0?})
	runtime/panic.go:1047 +0x65 fp=0xc000e13b88 sp=0xc000e13b58 pc=0x2197c5
runtime.sigpanic()
	runtime/signal_windows.go:249 +0x213 fp=0xc000e13bd0 sp=0xc000e13b88 pc=0x22cd53
runtime.memclrNoHeapPointers()
	runtime/memclr_amd64.s:182 +0x224 fp=0xc000e13bd8 sp=0xc000e13bd0 pc=0x2487c4
runtime.mallocgc(0x18, 0xad9860, 0x1)
	runtime/malloc.go:1022 +0x526 fp=0xc000e13c50 sp=0xc000e13bd8 pc=0x1ed9e6
runtime.convTslice({0xc000e44a20, 0x3, 0x3})
	runtime/iface.go:403 +0x4a fp=0xc000e13c78 sp=0xc000e13c50 pc=0x1eb6aa
github.com/iamacarpet/go-win64api.ListLoggedInUsers()
	github.com/iamacarpet/[email protected]/sessions.go:82 +0x3f0 fp=0xc000e13eb8 sp=0xc000e13c78 pc=0x6e1c90
github.com/Notifiarr/notifiarr/pkg/snapshot.(*Snapshot).GetUsers(0xc00063b340, {0xc000e13f60?, 0xc000e13f38?})
	github.com/Notifiarr/notifiarr/pkg/snapshot/users_windows.go:15 +0x25 fp=0xc000e13f00 sp=0xc000e13eb8 pc=0x6eeae5
github.com/Notifiarr/notifiarr/pkg/snapshot.(*Snapshot).GetLocalData(0xc00063b340, {0xd4d048, 0xc00124aea0})
	github.com/Notifiarr/notifiarr/pkg/snapshot/system.go:76 +0xec fp=0xc000e13f98 sp=0xc000e13f00 pc=0x6ed98c
github.com/Notifiarr/notifiarr/pkg/triggers/plexcron.(*cmd).getMetaSnap.func3()
	github.com/Notifiarr/notifiarr/pkg/triggers/plexcron/plexcron.go:167 +0x65 fp=0xc000e13fe0 sp=0xc000e13f98 pc=0x947a85
runtime.goexit()
	runtime/asm_amd64.s:1594 +0x1 fp=0xc000e13fe8 sp=0xc000e13fe0 pc=0x247a01
created by github.com/Notifiarr/notifiarr/pkg/triggers/plexcron.(*cmd).getMetaSnap
	github.com/Notifiarr/notifiarr/pkg/triggers/plexcron/plexcron.go:164 +0x1f8