From b7cc17f20a468d7a6cefe66ae85f34043ef037f0 Mon Sep 17 00:00:00 2001
From: Ivan Sorokin <vanyacpp@gmail.com>
Date: Tue, 28 Feb 2023 21:15:53 +0100
Subject: [PATCH] fix memory leak of %rep mmacro

When running with -fsanitize=leak enabled nasm prints this error:

Direct leak of 960 byte(s) in 5 object(s) allocated from:
    #0 0x7f52b6464a37 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x55cf36676c46 in nasm_calloc nasmlib/alloc.c:72
    #2 0x55cf36676cd1 in nasm_zalloc nasmlib/alloc.c:87
    #3 0x55cf366e3980 in do_directive asm/preproc.c:4754
    #4 0x55cf366fec97 in pp_tokline asm/preproc.c:7773
    #5 0x55cf366ff84a in pp_getline asm/preproc.c:7837
    #6 0x55cf3667263c in assemble_file asm/nasm.c:1722
    #7 0x55cf3666b4e4 in main asm/nasm.c:719
    #8 0x7f52b5b7cd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #9 0x7f52b5b7ce3f in __libc_start_main_impl ../csu/libc-start.c:392
    #10 0x55cf36666e04 in _start (/home/ivan/d/nasm/nasm+0x2e2e04)

This error is reproducible on lnxlinux.asm test or on this
small snippet:

    %rep 8
        nop
        nop
        nop
    %endrep

The original call to free_mmacro was commented out in
91e72409bec0910456e1c34457a04ca00e2f7b99 as it caused
use-after-free. https://bugzilla.nasm.us/show_bug.cgi?id=3392414

After adding free_mmacro I tested nasm with -fsanitize=address
on all four reproducers attached to the issue and none of them
causes use-after-free now. Also this commit passes all tests
without causing use-after-free.

Signed-off-by: Ivan Sorokin <vanyacpp@gmail.com>
---
 asm/preproc.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/asm/preproc.c b/asm/preproc.c
index ac42131e9..cb6f883fa 100644
--- a/asm/preproc.c
+++ b/asm/preproc.c
@@ -7594,6 +7594,7 @@ static Token *pp_tokline(void)
                 break;
             } else {
                 MMacro *m = istk->mstk.mstk;
+                bool should_free_mmacro = false;
 
                 /*
                  * Check whether a `%rep' was started and not ended
@@ -7648,6 +7649,8 @@ static Token *pp_tokline(void)
 			m->paramlen = NULL;
                     }
                 }
+                else
+                    should_free_mmacro = true;
 
                 if (fm->nolist & NL_LINE) {
                     istk->noline--;
@@ -7667,16 +7670,8 @@ static Token *pp_tokline(void)
 
                 istk->where = l->where;
 
-                /*
-                 * FIXME It is incorrect to always free_mmacro here.
-                 * It leads to usage-after-free.
-                 *
-                 * https://bugzilla.nasm.us/show_bug.cgi?id=3392414
-                 */
-#if 0
-                else
+                if (should_free_mmacro)
                     free_mmacro(m);
-#endif
             }
             istk->expansion = l->next;
             nasm_free(l);