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

[hpa at zytor.com]

On December 6, 2020 8:04:25 AM PST, Igor Munkin <imun at cpan.org> wrote:
>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

Hi,

I think we're should simply disallow undefining or redefining a macro that is in the process of being expanded. We have that information as we tag all the macros we have descended through.

I think this can be an error and the %unmacro dropped with a nasm_nonfatal() error message.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.