public inbox for ~johnnyrichard/olang-devel@lists.sr.ht
 help / color / mirror / code / Atom feed
From: Carlos Maniero <carlos@maniero.me>
To: ~johnnyrichard/olang-devel@lists.sr.ht
Cc: Carlos Maniero <carlos@maniero.me>
Subject: [PATCH olang] fix: codegen: prevent stack overwrite
Date: Tue, 15 Oct 2024 12:14:08 +0000 (UTC)	[thread overview]
Message-ID: <20241015121404.206543-1-carlos@maniero.me> (raw)

There was an issue in the stack allocation algorithm. Consider this
function:

  fn a(): u32 {
    var a: u32 = 0xAAAA
    var b: u64 = 0xBBBBBBBB

    ret
  }

There are three information the stack is required to store:

- 8 bytes: rip (from call instruction)
- 4 bytes: a
- 8 bytes: b

The 0x7FFFFF07 memory address was used to represent the RIP value at
call instant.

Our codegen was assuming the stack works that way:

0       -8  -C
^-------^---^-------
7FFFFF07AAAABBBBBBBB
^-------^---^-------
rip     a   b

So the code gen was:

- Adding the value at the stack;
- Increasing the offset.

But actually the stack was behaving as following:

8       0       -8  -C
^-------^-------^---^
7FFFFF070000BBBBBBBB.
^---------------^---^
rip             a   b

Once the instruction *mov %rax, -0xC(%rbp)* writes from -0xC(%rbp)
(exclusive) to -0x4(%rbp) (inclusive).

So after this change, this is the actual stack template:

        0   -4      -C
--------^---^-------^
7FFFFF07AAAABBBBBBBB.
--------^---^-------^
      rip   a       b

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/codegen_linux_x86_64.c          | 13 ++++++-------
 tests/olc/0036_variable_overflow.ol | 30 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 7 deletions(-)
 create mode 100644 tests/olc/0036_variable_overflow.ol

diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
index fc8fcc4..83d1d2c 100644
--- a/src/codegen_linux_x86_64.c
+++ b/src/codegen_linux_x86_64.c
@@ -28,7 +28,6 @@
 
 // The call instruction pushes EIP into stack so the first 8 bytes from stack
 // must be preserved else the ret instruction will jump to nowere.
-#define X86_CALL_EIP_STACK_OFFSET (8)
 #define X86_CALL_ARG_SIZE 6
 
 #define bytes_max(a, b) ((a) > (b) ? (a) : (b))
@@ -795,6 +794,9 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
                 symbol_t *symbol = scope_lookup(scope, var_def.id);
                 assert(symbol);
 
+                size_t type_size = type_to_bytes(symbol->type);
+                codegen->base_offset += type_size;
+
                 codegen_linux_x86_64_put_stack_offset(
                     codegen, symbol, codegen->base_offset);
 
@@ -803,13 +805,10 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
                                                          var_def.value);
                 }
 
-                size_t type_size = type_to_bytes(symbol->type);
-
                 fprintf(codegen->out,
                         "    mov %s, -%ld(%%rbp)\n",
                         get_reg_for(REG_ACCUMULATOR, type_size),
                         codegen->base_offset);
-                codegen->base_offset += type_size;
 
                 break;
             }
@@ -957,7 +956,7 @@ static void
 codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen,
                                    ast_fn_definition_t *fn_def)
 {
-    codegen->base_offset = X86_CALL_EIP_STACK_OFFSET;
+    codegen->base_offset = 0;
 
     ast_node_t *block_node = fn_def->block;
     fprintf(codegen->out, "" SV_FMT ":\n", SV_ARG(fn_def->id));
@@ -975,6 +974,8 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen,
         symbol_t *symbol = scope_lookup(fn_def->scope, param->id);
         assert(symbol);
 
+        // FIXME: add offset according to the param size
+        codegen->base_offset += 8;
         size_t offset = codegen->base_offset;
 
         codegen_linux_x86_64_put_stack_offset(
@@ -986,8 +987,6 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen,
                 get_reg_for(x86_call_args[i], symbol->type->as_primitive.size),
                 offset);
 
-        // FIXME: add offset according to the param size
-        codegen->base_offset += 8;
         ++i;
     }
 
diff --git a/tests/olc/0036_variable_overflow.ol b/tests/olc/0036_variable_overflow.ol
new file mode 100644
index 0000000..edb3c7e
--- /dev/null
+++ b/tests/olc/0036_variable_overflow.ol
@@ -0,0 +1,30 @@
+# Copyright (C) 2024 olang mantainers
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+fn main(): u32 {
+  var a: u32 = 0
+  var b: u64 = 0
+  var c: u32 = 0
+
+  # This operation will fill all bits in b location.
+  # If there is an overflow, both a or c would be impacted
+  b = ~b
+
+  return a + c
+}
+
+# TEST test_compile(exit_code=0)
+
+# TEST test_run_binary(exit_code=0)

base-commit: cf5e4abf07a38f0ddf3ac6979b01b942ab99a691
-- 
2.46.1


             reply	other threads:[~2024-10-15 12:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-15 12:14 Carlos Maniero [this message]
2024-10-15 12:14 ` [olang/patches/.build.yml] build failed builds.sr.ht
2024-10-15 23:03   ` Carlos Maniero
2024-10-16 22:33 ` [PATCH olang] fix: codegen: prevent stack overwrite Johnny Richard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241015121404.206543-1-carlos@maniero.me \
    --to=carlos@maniero.me \
    --cc=~johnnyrichard/olang-devel@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.johnnyrichard.com/olang.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox