Re: [FFmpeg-devel] [PATCH 5/5] checkasm: aarch64: Check for stack overflows

2020-05-15 Thread Martin Storsjö

On Fri, 15 May 2020, Henrik Gramner wrote:


All 5 lgtm.


Thanks, pushed.

// Martin

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/5] checkasm: aarch64: Check for stack overflows

2020-05-15 Thread Henrik Gramner
All 5 lgtm.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/5] checkasm: aarch64: Check for stack overflows

2020-05-14 Thread Martin Storsjö
Also fill x8-x17 with garbage before calling the function.

Figure out the number of stack parameters and make sure that the
value on the stack after those is untouched.
---
 tests/checkasm/aarch64/checkasm.S | 47 +--
 tests/checkasm/checkasm.h |  7 +++--
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/tests/checkasm/aarch64/checkasm.S 
b/tests/checkasm/aarch64/checkasm.S
index 0dbfe8025e..6d3c738801 100644
--- a/tests/checkasm/aarch64/checkasm.S
+++ b/tests/checkasm/aarch64/checkasm.S
@@ -44,8 +44,10 @@ const register_init, align=4
 endconst
 
 
-const error_message
+const error_message_register
 .asciz "failed to preserve register"
+error_message_stack:
+.asciz "stack clobbered"
 endconst
 
 
@@ -65,7 +67,8 @@ function checkasm_stack_clobber, export=1
 ret
 endfunc
 
-#define ARG_STACK ((8*(MAX_ARGS - 8) + 15) & ~15)
+// + 16 for stack canary reference
+#define ARG_STACK ((8*(MAX_ARGS - 8) + 15) & ~15 + 16)
 
 function checkasm_checked_call, export=1
 stp x29, x30, [sp, #-16]!
@@ -100,14 +103,48 @@ function checkasm_checked_call, export=1
 .equ pos, pos + 8
 .endr
 
+// Fill x8-x17 with garbage. This doesn't have to be preserved,
+// but avoids relying on them having any particular value.
+movrel  x9, register_init
+ldp x10, x11, [x9], #32
+ldp x12, x13, [x9], #32
+ldp x14, x15, [x9], #32
+ldp x16, x17, [x9], #32
+ldp x8,  x9,  [x9]
+
+// For stack overflows, the callee is free to overwrite the parameters
+// that were passed on the stack (if any), so we can only check after
+// that point. First figure out how many parameters the function
+// really took on the stack:
+ldr w2,  [x29, #16 + 8*8 + (MAX_ARGS-8)*8]
+// Load the first non-parameter value from the stack, that should be
+// left untouched by the function. Store a copy of it inverted, so that
+// e.g. overwriting everything with zero would be noticed.
+ldr x2,  [sp, x2, lsl #3]
+mvn x2,  x2
+str x2,  [sp, #ARG_STACK-8]
+
+// Load the in-register arguments
 mov x12, x0
 ldp x0,  x1,  [x29, #16]
 ldp x2,  x3,  [x29, #32]
 ldp x4,  x5,  [x29, #48]
 ldp x6,  x7,  [x29, #64]
+// Call the target function
 blr x12
+
+// Load the number of stack parameters, stack canary and its reference
+ldr w2,  [x29, #16 + 8*8 + (MAX_ARGS-8)*8]
+ldr x2,  [sp, x2, lsl #3]
+ldr x3,  [sp, #ARG_STACK-8]
+
 add sp,  sp,  #ARG_STACK
 stp x0,  x1,  [sp, #-16]!
+
+mvn x3,  x3
+cmp x2,  x3
+b.ne2f
+
 movrel  x9, register_init
 moviv3.8h,  #0
 
@@ -139,7 +176,11 @@ function checkasm_checked_call, export=1
 
 cbz x3,  0f
 
-movrel  x0, error_message
+movrel  x0, error_message_register
+b   1f
+2:
+movrel  x0, error_message_stack
+1:
 bl  X(checkasm_fail_func)
 0:
 ldp x0,  x1,  [sp], #16
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index 254e28f5e2..e7d47475f6 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -183,12 +183,15 @@ extern void (*checkasm_checked_call)(void *func, int 
dummy, ...);
 #elif ARCH_AARCH64 && !defined(__APPLE__)
 void checkasm_stack_clobber(uint64_t clobber, ...);
 void checkasm_checked_call(void *func, ...);
-#define declare_new(ret, ...) ret (*checked_call)(void *, int, int, int, int, 
int, int, int, __VA_ARGS__)\
+#define declare_new(ret, ...) ret (*checked_call)(void *, int, int, int, int, 
int, int, int, __VA_ARGS__,\
+  int, int, int, int, int, 
int, int, int,\
+  int, int, int, int, int, 
int, int)\
   = (void *)checkasm_checked_call;
 #define CLOB (UINT64_C(0xdeadbeefdeadbeef))
 #define call_new(...) 
(checkasm_stack_clobber(CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,\
   
CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB),\
-  checked_call(func_new, 0, 0, 0, 0, 0, 0, 0, __VA_ARGS__))
+  checked_call(func_new, 0, 0, 0, 0, 0, 0, 0, __VA_ARGS__,\
+   7, 6, 5, 4, 3, 2, 1, 0, 0, 0, 0, 0, 0, 0, 
0))
 #else
 #define declare_new(ret, ...)
 #define declare_new_float(ret, ...)
-- 
2.17.1

___