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.

Sign up for GitHub

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

Prevent early destruction of gFuns, fixes AddressSanitizer: heap-use-after-free #22386

Merged
merged 1 commit into from
Aug 5, 2023
Merged

Prevent early destruction of gFuns, fixes AddressSanitizer: heap-use-after-free #22386

merged 1 commit into from
Aug 5, 2023

Conversation

norrath-hero-cn
Copy link
Contributor

@norrath-hero-cn norrath-hero-cn commented Aug 4, 2023 •
edited
Loading

I now fully understand what is happening in #22082.

The real cause of the problem is that the destruction of global var gFuns in lib/std/exitprocs.nim is too early before the atexit call to callClosures.

With a few lines of code, I can now reproduce this problem in a stable way.

import std/exitprocs

var x {.cursor.}: seq[int]

x.add 1

proc dump() = echo "dump:", x #without cursor this line will crash

addExitProc(dump)

=================================================================
==4616==ERROR: AddressSanitizer: heap-use-after-free on address 0x127770220018 at pc 0x7ffafb2a1f47 bp 0x00f275b0ef50 sp 0x00f275b0ef98
READ of size 24 at 0x127770220018 thread T0
    #0 0x7ffafb2a1f46 in __asan_memcpy /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
    #1 0x7ff637584b10 in callClosures__stdZexitprocs_u19 D:\vscode\vscode_nim\nim-2.0.0\lib/system.nim:939:125
    #2 0x7ffb441242d5  (C:\WINDOWS\System32\ucrtbase.dll+0x1800142d5)
    #3 0x7ffb441241fa  (C:\WINDOWS\System32\ucrtbase.dll+0x1800141fa)
    #4 0x7ffb441241b3  (C:\WINDOWS\System32\ucrtbase.dll+0x1800141b3)
    #5 0x7ffb44130521  (C:\WINDOWS\System32\ucrtbase.dll+0x180020521)
    #6 0x7ffb441304aa  (C:\WINDOWS\System32\ucrtbase.dll+0x1800204aa)
    #7 0x7ffb4413044d  (C:\WINDOWS\System32\ucrtbase.dll+0x18002044d)
    #8 0x7ff63758134a in __tmainCRTStartup /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:269:7     
    #9 0x7ff637581365 in .l_start /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:188:9
    #10 0x7ffb448c7613  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017613)
    #11 0x7ffb468a26b0  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800526b0)

0x127770220018 is located 8 bytes inside of 32-byte region [0x127770220010,0x127770220030)
freed by thread T0 here:
    #0 0x7ffafb2a2671 in free /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:82:3
    #1 0x7ff637594483 in alignedDealloc D:\vscode\vscode_nim\nim-2.0.0\lib\system/memalloc.nim
    #2 0x7ff6375a4e96 in NimMainModule D:\vscode\vscode_nim\nim-2.0.0\lib\std/exitprocs.nim:26:2
    #3 0x7ff6375a50d2 in NimMainInner D:\vscode\vscode_nim\nim-2.0.0\lib\system/memory.nim:57:2
    #4 0x7ff6375a50d2 in NimMain D:\vscode\vscode_nim\nim-2.0.0\lib\system/memory.nim:68:2
    #5 0x7ff6375a50d2 in main D:\vscode\vscode_nim\nim-2.0.0\lib\system/memory.nim:76:2
    #6 0x7ff637581314 in __tmainCRTStartup /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:267:15    
    #7 0x7ff637581365 in .l_start /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:188:9
    #8 0x7ffb448c7613  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017613)
    #9 0x7ffb468a26b0  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800526b0)

previously allocated by thread T0 here:
    #0 0x7ffafb2a28a6 in calloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:114:3
    #1 0x7ff63759d7ac in alloc0Impl__system_u1763 D:\vscode\vscode_nim\nim-2.0.0\lib\system\mm/malloc.nim:11:11
    #2 0x7ff63759d7ac in allocShared0Impl__system_u1776 D:\vscode\vscode_nim\nim-2.0.0\lib\system\mm/malloc.nim:37:11
    #3 0x7ff63759d7ac in alignedAlloc0__system_u1947 D:\vscode\vscode_nim\nim-2.0.0\lib\system/memalloc.nim:351:12
    #4 0x7ff63759e356 in newSeqPayload D:\vscode\vscode_nim\nim-2.0.0\lib\system/seqs_v2.nim:44:773
    #5 0x7ff63759f2a0 in prepareSeqAdd D:\vscode\vscode_nim\nim-2.0.0\lib\system/seqs_v2.nim:62:122
    #6 0x7ff63759fa37 in add__stdZexitprocs_u226 D:\vscode\vscode_nim\nim-2.0.0\lib\system/seqs_v2.nim:116:26
    #7 0x7ff637585716 in addExitProc__stdZexitprocs_u220 D:\vscode\vscode_nim\nim-2.0.0\lib\std/exitprocs.nim:65:2
    #8 0x7ff6375a4e2b in NimMainModule D:\vscode\vscode_nim\projects\sanitizer\src/sanitizer.nim:7:161
    #9 0x7ff6375a50d2 in NimMainInner D:\vscode\vscode_nim\nim-2.0.0\lib\system/memory.nim:57:2
    #10 0x7ff6375a50d2 in NimMain D:\vscode\vscode_nim\nim-2.0.0\lib\system/memory.nim:68:2
    #11 0x7ff6375a50d2 in main D:\vscode\vscode_nim\nim-2.0.0\lib\system/memory.nim:76:2
    #12 0x7ff637581314 in __tmainCRTStartup /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:267:15   
    #13 0x7ff637581365 in .l_start /home/runner/work/llvm-mingw/llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64/../crt/crtexe.c:188:9
    #14 0x7ffb448c7613  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017613)
    #15 0x7ffb468a26b0  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800526b0)

SUMMARY: AddressSanitizer: heap-use-after-free /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 in __asan_memcpy
Shadow bytes around the buggy address:
  0x12777021fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12777021fe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12777021fe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12777021ff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x12777021ff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x127770220000: fa fa fd[fd]fd fd fa fa fa fa fa fa fa fa fa fa
  0x127770220080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x127770220100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x127770220180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x127770220280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==4616==ABORTING

After this patch, AddressSanitizer: heap-use-after-free no longer exists. this fixes the problem for both 1.6.x and 2.0.0.

@norrath-hero-cn norrath-hero-cn changed the title Prevent destruction of gFuns before callClosures Prevent early destruction of gFuns, fixes AddressSanitizer: heap-use-after-free Aug 5, 2023
@norrath-hero-cn norrath-hero-cn mentioned this pull request Aug 5, 2023
Closed
@Araq Araq merged commit e039690 into nim-lang:devel Aug 5, 2023
19 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2023

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from e039690

Hint: mm: orc; opt: speed; options: -d:release
168493 lines; 11.815s; 611.668MiB peakmem

@norrath-hero-cn norrath-hero-cn deleted the addExitProc-fix branch August 6, 2023 01:21
narimiran pushed a commit that referenced this pull request Apr 17, 2024
@norrath-hero-cn @narimiran
Prevent early destruction of gFuns, fixes AddressSanitizer: heap-use-…
36b339b 
…after-free (#22386)

Prevent destruction of gFuns before callClosures

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

Successfully merging this pull request may close these issues.
2 participants
@norrath-hero-cn @Araq