DynamoRIO / dynamorio

Dynamic Instrumentation Tool Platform
Other
2.67k stars 562 forks source link

dynamorio could crash when clone syscall's newsp == 0 #4043

Open hac425xxx opened 4 years ago

hac425xxx commented 4 years ago

when newsp == 0, we should save the mc->sp to record->app_thread_xsp

Bug fix

core/unix/signal.c

@@ -632,6 +632,13 @@ create_clone_record(dcontext_t *dcontext, reg_t *app_thread_xsp)
        record = (clone_record_t *)(dstack - sizeof(clone_record_t));
        ASSERT(ALIGNED(record, get_ABI_stack_alignment()));
        record->app_thread_xsp = *app_thread_xsp;

        /* fix bug when  clone syscall's argument: newsp == 0  (for example: vfork in android libc.so)
         */
        if(record->app_thread_xsp == 0)
        {
            record->app_thread_xsp = get_mcontext(dcontext)->sp;
        }
        /* asynch_target is set in d_r_dispatch() prior to calling pre_system_call(). */
        record->continuation_pc = dcontext->asynch_target;
        record->clone_flags = dcontext->sys_param0;
AssadHashmi commented 4 years ago

Thanks for pointing this out @hac425xxx. We'll look at this. Do you have a test case which exposed this and did the change fix it? (The comment mentions 'vfork in android libc.so')

hac425xxx commented 4 years ago

Yes, this patch fix the bug.

the testcase: vfork function in libc.so (android 9 and android 10)

hac425xxx commented 4 years ago

vfork function’s assembly code

.text:000000000001F300                 WEAK vfork
.text:000000000001F300 vfork                                   ; CODE XREF: .vfork+C↑j
.text:000000000001F300                                         ; DATA XREF: LOAD:00000000000079A8↑o ...
.text:000000000001F300 ; __unwind {
.text:000000000001F300                 MRS             X0, #3, c13, c0, #2
.text:000000000001F304                 LDR             X0, [X0,#8]
.text:000000000001F308                 STR             WZR, [X0,#0x14]
.text:000000000001F30C                 MOV             X0, #0x4111
.text:000000000001F310                 MOV             X1, XZR
.text:000000000001F314                 MOV             X2, XZR
.text:000000000001F318                 MOV             X3, XZR
.text:000000000001F31C                 MOV             X4, XZR
.text:000000000001F320                 MOV             X8, #0xDC
.text:000000000001F324                 SVC             0
.text:000000000001F328                 CMN             X0, #1,LSL#12
.text:000000000001F32C                 CINV            X0, X0, HI
.text:000000000001F330                 B.HI            __set_errno_internal
.text:000000000001F334                 RET
.text:000000000001F334 ; } // starts at 1F300
.text:000000000001F334 ; End of function vfork
hac425xxx commented 4 years ago

@AssadHashmi