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

Jump to bottom

Memory Error in vibe.utils.memory / FreeListRef: Refcount is stored outside allocated Memory #1432

Closed
tosttost opened this issue Feb 19, 2016 · 1 comment
Closed

Memory Error in vibe.utils.memory / FreeListRef: Refcount is stored outside allocated Memory #1432

tosttost opened this issue Feb 19, 2016 · 1 comment

Comments

@tosttost
Copy link
Contributor

I discovered this issue while using valgrind. On serving a file using serveStaticFile valgrind reports an invalid read of size 4 whenever the refcount of a FreeListRef is accessed.

I read the source code. It seems that the "+ int.sizeof" to allocate memory for storing ref count got lost with commit 6be5471 .
As far as I figured out:

  • the memory for FreeListRef is allocated by calling FreeListObjectAlloc
  • FreeListObjectAlloc allocates AllocSize!T bytes (line 639 / 616)
  • AllocSize is just the size of T (or the classInstanceSize if T is a class), so no memory for storing the refcount is allocated (line 672)

I have no time to figure out where the "+ int.sizeof" fits best (and therefore, can't create a PR).

Valgrind output:
==5411== Invalid write of size 4
==5411== at 0xC97B52: D4vibe5utils6memory110T11FreeListRefTS4vibe4core6stream12OutputStream12writeDefaultMFC4vibe4core6stream11InputStreammZ6BufferVbi0Z11FreeListRef11T6opCallZ6opCallFNbZS4vibe5utils6memory110T11FreeListRefTS4vibe4core6stream12OutputStream12writeDefaultMFC4vibe4core6stream11InputStreammZ6BufferVbi0Z11FreeListRef (../../.dub/packages/vibe-d-0.7.27/source/vibe/utils/memory.d:694)
==5411== Address 0x1899e270 is 0 bytes after a block of size 65,552 alloc'd
==5411== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreloadmemcheck-amd64-linux.so)
==5411==
==5411== Invalid read of size 4
==5411== at 0xC97AC2: D4vibe5utils6memory110T11FreeListRefTS4vibe4core6stream12OutputStream12writeDefaultMFC4vibe4core6stream11InputStreammZ6BufferVbi0Z11FreeListRef15checkInvariantsMxFNaNbNiZv (../../.dub/packages/vibe-d-0.7.27/source/vibe/utils/memory.d:753)
==5411== Address 0x1899e270 is 0 bytes after a block of size 65,552 alloc'd
==5411== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreloadmemcheck-amd64-linux.so)
==5411==
==5411== Invalid read of size 4
==5411== at 0xC97988: D4vibe5utils6memory110T11FreeListRefTS4vibe4core6stream12OutputStream12writeDefaultMFC4vibe4core6stream11InputStreammZ6BufferVbi0Z11FreeListRef5clearMFNbNiZv (../../.dub/packages/vibe-d-0.7.27/source/vibe/utils/memory.d:732)
==5411== Address 0x1899e270 is 0 bytes after a block of size 65,552 alloc'd
==5411== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreloadmemcheck-amd64-linux.so)
==5411==
==5411== Invalid read of size 4
==5411== at 0xC9798A: D4vibe5utils6memory110T11FreeListRefTS4vibe4core6stream12OutputStream12writeDefaultMFC4vibe4core6stream11InputStreammZ6BufferVbi0Z11FreeListRef5clearMFNbNiZv (../../.dub/packages/vibe-d-0.7.27/source/vibe/utils/memory.d:732)
==5411== Address 0x1899e270 is 0 bytes after a block of size 65,552 alloc'd
==5411== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreloadmemcheck-amd64-linux.so)
@s-ludwig
Copy link
Member

Thanks for taking this to GitHub. I saw the thread on the forum, but didn't manage to look into it, yet.

I'll make a quick bugfix release soon.

@s-ludwig s-ludwig mentioned this issue Feb 20, 2016
Closed
s-ludwig added a commit that referenced this issue Feb 20, 2016
@s-ludwig
Fix regression in FreeListRef where no memory was allocated for the r…
613d159 
…ef count. Fixes #1432.

Commit 6be5471 switched FreeListRef to use FreeListObjectAlloc underneath, but didn't accound for the extra memory that is needed to store the reference count directly after the object payload. The possible implications of this are memory corruption and memory leaks, although with the predefined allocator setip, this will only happen to types with a POT size or slightly less.

This commit adds an "EXTRA" template type parameter to FreeListObjectAlloc that is used to determine the additional amount of allocated memory, which is set to "int" in the case of FreeListRef.

(cherry picked from commit d78a9ce)
s-ludwig added a commit that referenced this issue Apr 10, 2016
@s-ludwig
Fix regression in FreeListRef where no memory was allocated for the r…
422c202 
…ef count. Fixes #1432.

Commit 6be5471 switched FreeListRef to use FreeListObjectAlloc underneath, but didn't accound for the extra memory that is needed to store the reference count directly after the object payload. The possible implications of this are memory corruption and memory leaks, although with the predefined allocator setip, this will only happen to types with a POT size or slightly less.

This commit adds an "EXTRA" template type parameter to FreeListObjectAlloc that is used to determine the additional amount of allocated memory, which is set to "int" in the case of FreeListRef.

(cherry picked from commit d78a9ce)

(cherry picked from commit 613d159)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@s-ludwig @tosttost