[Nasm-devel] [Bug 3392716] Macro expansion behaviour

[Igor Munkin]

Hi, Peter!

Cyrill some time ago suggested me to switch onto this bug[1], to dive
more into NASM frontend and preprocessor.

At first, I tried to reduce the reproducer attached to the ticket, and
got the one below:
| $ cat mini-3392716.asm
| %macro sst
| %unmacro sst
| %endmacro
| sst

Strictly saying, it reproduces another problem (another heap use after
free is reported by ASAN), but I guess the root cause is the similar.
Anyway if one runs the snipped above with NASM compiled with sanitizers,
the following error is reported:
| $ ./configure --enable-sanitizer --enable-gdb --disable-optimization
| <snipped>
| $ make -j
| <snipped>
| $ ./nasm -fmacho64 -g -o xxx mini-3392716.asm
| mini-3392716.asm:1: error: `%macro' expects a parameter count
| mini-3392716.asm:4: error: `%unmacro' expects a parameter count
| mini-3392716.asm:2: ... from macro `sst' defined here
| =================================================================
| ==7426==ERROR: AddressSanitizer: heap-use-after-free on address 0x610000001f48 at pc 0x5594217fe7ef bp 0x7ffebfbf7ea0 sp 0x7ffebfbf7e90
| READ of size 8 at 0x610000001f48 thread T0
|     #0 0x5594217fe7ee in pp_tokline asm/preproc.c:7056
|     #1 0x559421801900 in pp_getline asm/preproc.c:7309
|     #2 0x55942177ecf4 in assemble_file asm/nasm.c:1722
|     #3 0x559421777ed4 in main asm/nasm.c:717
|     #4 0x7f4f81adbeda in __libc_start_main (/lib64/libc.so.6+0x23eda)
|     #5 0x559421773729 in _start (/home/imun/projects/nasm/nasm+0x2bc729)
|
| 0x610000001f48 is located 8 bytes inside of 192-byte region [0x610000001f40,0x610000002000)
| freed by thread T0 here:
|     #0 0x7f4f826e750f in __interceptor_free /var/tmp/portage/sys-devel/gcc-9.3.0-r1/work/gcc-9.3.0/libsanitizer/asan/asan_malloc_linux.cc:122
|     #1 0x55942178312b in nasm_free nasmlib/alloc.c:108
|     #2 0x5594217ccf97 in free_mmacro asm/preproc.c:958
|     #3 0x5594217e8240 in do_directive asm/preproc.c:4341
|     #4 0x559421800dea in pp_tokline asm/preproc.c:7245
|     #5 0x559421801900 in pp_getline asm/preproc.c:7309
|     #6 0x55942177ecf4 in assemble_file asm/nasm.c:1722
|     #7 0x559421777ed4 in main asm/nasm.c:717
|     #8 0x7f4f81adbeda in __libc_start_main (/lib64/libc.so.6+0x23eda)
|
| previously allocated by thread T0 here:
|     #0 0x7f4f826e7af8 in __interceptor_calloc /var/tmp/portage/sys-devel/gcc-9.3.0-r1/work/gcc-9.3.0/libsanitizer/asan/asan_malloc_linux.cc:153
|     #1 0x559421782fdf in nasm_calloc nasmlib/alloc.c:72
|     #2 0x559421783061 in nasm_zalloc nasmlib/alloc.c:87
|     #3 0x5594217e68db in do_directive asm/preproc.c:4246
|     #4 0x559421800dea in pp_tokline asm/preproc.c:7245
|     #5 0x559421801900 in pp_getline asm/preproc.c:7309
|     #6 0x55942177ecf4 in assemble_file asm/nasm.c:1722
|     #7 0x559421777ed4 in main asm/nasm.c:717
|     #8 0x7f4f81adbeda in __libc_start_main (/lib64/libc.so.6+0x23eda)
|
| SUMMARY: AddressSanitizer: heap-use-after-free asm/preproc.c:7056 in pp_tokline
| Shadow bytes around the buggy address:
|   0x0c207fff8390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x0c207fff83a0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
|   0x0c207fff83b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
|   0x0c207fff83c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
|   0x0c207fff83d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
| =>0x0c207fff83e0: fa fa fa fa fa fa fa fa fd[fd]fd fd fd fd fd fd
|   0x0c207fff83f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
|   0x0c207fff8400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x0c207fff8410: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x0c207fff8420: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
|   0x0c207fff8430: 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
|   ASan internal:           fe
|   Left alloca redzone:     ca
|   Right alloca redzone:    cb
|   Shadow gap:              cc
| ==7426==ABORTING

I interactively debugged this and it occurs, that the macro being
currently expanded has just been undefined in scope of this expansion
and this leads to invalid read of already freed memory. AFAICS there are
no constraints for such operation (omitting the warnings reported
regarding improper macro definition syntax), so it's quite legit to
undefine the macro being expanded implementing kinda "oneshot" macro.

As a result of offline discussion with Cyrill we haven't come to the
final solution, but agreed that cleanup made in scope of <PP_UNMACRO>
has to be deferred prior to macro expansion is finished and such oneshot
definitions should be handled the following way:
| $ cat oneshot.pl
| use 5.010;
|
| my $oneshot; $oneshot = sub {
|   say "QQ";
|   undef $oneshot;
| };
|
| &$oneshot;
| &$oneshot;
| $ perl oneshot.pl
| QQ
| Can't use an undefined value as a subroutine reference at oneshot.pl line 9.

At this point we need to make the design-related decision prior to
fixing the issue and I believe we can't make one without you.

[1]: https://bugzilla.nasm.us/show_bug.cgi?id=3392716

-- 
Best regards,
IM