Patchwork Process record: Fix null deref when loading empty core file

login
register
mail settings
Submitter Andrew D'Addesio
Date May 11, 2018, 9:54 p.m.
Message ID <1526075698-20880-1-git-send-email-modchipv12@gmail.com>
Download mbox | patch
Permalink /patch/27252/
State New
Headers show

Comments

Andrew D'Addesio - May 11, 2018, 9:54 p.m.
Fix a null dereference in the "record full restore" command. If the
supplied file contains no records, the arch list will be empty, so
no need to copy to the record list.

Also remove a redundant "record_full_arch_list_tail->next = NULL;"
assignment, as our arch list is already non-circular by design.

gdb/ChangeLog:
2018-05-11  Andrew D'Addesio  <modchipv12@gmail.com>

	* record-full.c (record_full_restore): Avoid null deref when
	appending the arch list to the record list.
---
 gdb/record-full.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
Andrew D'Addesio - May 11, 2018, 10 p.m.
Here's an explanation of the bug:

Description:

gdb segfaults (null dereference) if the user attempts to run the command
"record full restore" on a file containing an empty execution history
log.

Steps to reproduce the bug:

1. Compile the following hello world using:
gcc -Wall -nostartfiles -o helloworld helloworld.S

    #include <asm/unistd.h>

    .intel_syntax noprefix
    .global _start

    .data
    msg:
        .ascii "hello, world!\n"
    msg_end:

    .text
    _start:
        mov rax, __NR_write
        mov rdi, 1 # STDOUT_FILENO
        lea rsi, [rip + msg]
        mov rdx, (msg_end - msg)
        syscall
        mov rax, __NR_exit
        mov rdi, 0 # EXIT_SUCCESS
        syscall

2. Launch gdb using: gdb ./helloworld

3. Execute these commands:

    break _start
    run
    record full

    # Uncommenting the next line prevents the crash:
    # stepi

    record save foo.log
    record stop
    record full restore foo.log # Segfault!

gdb terminates with the following segfault:

Program received signal SIGSEGV, Segmentation fault.
0x00000000007ff697 in record_full_restore () at
../../binutils-gdb/gdb/record-full.c:2491
2491      record_full_arch_list_head->prev = &record_full_first;
(gdb) backtrace
#0  0x00000000007ff697 in record_full_restore () at
../../binutils-gdb/gdb/record-full.c:2491
#1  0x00000000007fcade in record_full_core_open_1 (name=0x1e5af34
"foo.log", from_tty=1) at ../../binutils-gdb/gdb/record-full.c:940
#2  0x00000000007fcc3a in record_full_open (name=0x1e5af34 "foo.log",
from_tty=1) at ../../binutils-gdb/gdb/record-full.c:984
#3  0x00000000007ffbb3 in cmd_record_full_restore (args=0x1e5af34
"foo.log", from_tty=1) at ../../binutils-gdb/gdb/record-full.c:2532
#4  0x000000000048078c in do_const_cfunc (c=0x1d55830, args=0x1e5af34
"foo.log", from_tty=1) at ../../binutils-gdb/gdb/cli/cli-decode.c:106
#5  0x0000000000483884 in cmd_func (cmd=0x1d55830, args=0x1e5af34
"foo.log", from_tty=1) at ../../binutils-gdb/gdb/cli/cli-decode.c:1857
#6  0x00000000008b094f in execute_command (p=0x1e5af3a "g",
from_tty=1) at ../../binutils-gdb/gdb/top.c:630
#7  0x000000000070288c in command_handler (command=0x1e5af20 "") at
../../binutils-gdb/gdb/event-top.c:583
#8  0x0000000000702c90 in command_line_handler (rl=0x1e39190 "record
full restore foo.log") at ../../binutils-gdb/gdb/event-top.c:774
#9  0x0000000000702020 in gdb_rl_callback_handler (rl=0x1e39190
"record full restore foo.log") at
../../binutils-gdb/gdb/event-top.c:213
#10 0x00007fbda62b76f5 in rl_callback_read_char () from
/lib/x86_64-linux-gnu/libreadline.so.6
#11 0x0000000000701f0c in gdb_rl_callback_read_char_wrapper_noexcept
() at ../../binutils-gdb/gdb/event-top.c:175
#12 0x0000000000701f8b in gdb_rl_callback_read_char_wrapper
(client_data=0x1b63420) at ../../binutils-gdb/gdb/event-top.c:192
#13 0x0000000000702720 in stdin_event_handler (error=0,
client_data=0x1b63420) at ../../binutils-gdb/gdb/event-top.c:511
#14 0x00000000007007ed in handle_file_event (file_ptr=0x1e63d50,
ready_mask=1) at ../../binutils-gdb/gdb/event-loop.c:733
#15 0x0000000000700d9f in gdb_wait_for_event (block=1) at
../../binutils-gdb/gdb/event-loop.c:859
#16 0x00000000006ffbc7 in gdb_do_one_event () at
../../binutils-gdb/gdb/event-loop.c:347
#17 0x00000000006ffc0e in start_event_loop () at
../../binutils-gdb/gdb/event-loop.c:371
#18 0x0000000000793909 in captured_command_loop () at
../../binutils-gdb/gdb/main.c:330
#19 0x0000000000794def in captured_main (data=0x7fffc2ce81e0) at
../../binutils-gdb/gdb/main.c:1157
#20 0x0000000000794ec4 in gdb_main (args=0x7fffc2ce81e0) at
../../binutils-gdb/gdb/main.c:1173
#21 0x000000000040df4c in main (argc=2, argv=0x7fffc2ce82e8) at
../../binutils-gdb/gdb/gdb.c:32

This patch fixes the segfault. I also attached a passing testsuite.

Andrew
1c1
< Test Run By daddesio on Fri May 11 16:16:55 2018
---
> Test Run By daddesio on Fri May 11 16:29:35 2018
45265c45265
< FAIL: gdb.mi/list-thread-groups-available.exp: list available thread groups (unexpected output)
---
> PASS: gdb.mi/list-thread-groups-available.exp: list available thread groups
48105c48105
< FAIL: gdb.multi/multi-arch-exec.exp: first_arch=1: selected_thread=1: follow_exec_mode=same: continue across exec that changes architecture
---
> FAIL: gdb.multi/multi-arch-exec.exp: first_arch=1: selected_thread=1: follow_exec_mode=same: continue across exec that changes architecture (timeout)
56084c56084
< PASS: gdb.threads/non-ldr-exit.exp: program exits normally
---
> KFAIL: gdb.threads/non-ldr-exit.exp: program exits normally (PRMS: gdb/18717)
56703,56704c56703
< PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=1: inferior 1 exited
< PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=1: no threads left
---
> KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=1: inferior 1 exited (memory error) (PRMS: gdb/18749)
56708,56709c56707
< PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=0: inferior 1 exited
< PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=0: no threads left
---
> KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=on: cond_bp_target=0: inferior 1 exited (memory error) (PRMS: gdb/18749)
56718,56719c56716
< PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: inferior 1 exited
< PASS: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: no threads left
---
> KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: inferior 1 exited (prompt) (PRMS: gdb/18749)
57621c57618
< PASS: gdb.threads/thread-specific-bp.exp: non-stop: thread-specific breakpoint was deleted
---
> FAIL: gdb.threads/thread-specific-bp.exp: non-stop: thread-specific breakpoint was deleted (timeout)
58382c58379
< # of expected passes		54575
---
> # of expected passes		54568
58386c58383
< # of known failures		62
---
> # of known failures		66

Patch

diff --git a/gdb/record-full.c b/gdb/record-full.c
index 79f5c0f..edd30fb 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -2486,11 +2486,13 @@  record_full_restore (void)
 
   discard_cleanups (old_cleanups);
 
-  /* Add record_full_arch_list_head to the end of record list.  */
-  record_full_first.next = record_full_arch_list_head;
-  record_full_arch_list_head->prev = &record_full_first;
-  record_full_arch_list_tail->next = NULL;
-  record_full_list = &record_full_first;
+  /* Append the arch list to the record list.  */
+  if (record_full_arch_list_head != NULL)
+    {
+      record_full_first.next = record_full_arch_list_head;
+      record_full_arch_list_head->prev = &record_full_first;
+      record_full_list = &record_full_first;
+    }
 
   /* Update record_full_insn_max_num.  */
   if (record_full_insn_num > record_full_insn_max_num)