Message ID | 1526075698-20880-1-git-send-email-modchipv12@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 91752 invoked by alias); 11 May 2018 21:55:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 91731 invoked by uid 89); 11 May 2018 21:55:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=2.7.4, our X-HELO: mail-ot0-f170.google.com Received: from mail-ot0-f170.google.com (HELO mail-ot0-f170.google.com) (74.125.82.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 May 2018 21:55:36 +0000 Received: by mail-ot0-f170.google.com with SMTP id g7-v6so7865586otj.11 for <gdb-patches@sourceware.org>; Fri, 11 May 2018 14:55:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=H9obqFnL+YNWpbI+B1AHd5Z0XNnRcR6c+L34aWxpk7Q=; b=n8QUTzqO3nC+caKxbo6QOpkAuwpOatMyjPq6U0inblg+WO2Q1Ui/2nxutGIaJtNG2G gIr3XDpYxUP+IW43XWHwODYuQEk4oqhkfXSbo/TCGxaeSe3XAi0cjOmC8+dQC/UsywzP r8hFUPOWyrpX8cgIfd9Ws54nKpQrEeoha5ecI8FoXt0xBJGNNS/k9xPF5oUwVTWRjlUi a0WjlAUg/z4RTIMh1PguiyDBlOaKTKXFpkVjXwA2ENt6/EX99d4X6MyNHdoyTHruBDGt 9/gqweV1sD7+pyMBhN18gJjqyA+pau+2X1I5+aZy/jezKDqX4fMvl75mVzDx6TSQP1Hl c3Aw== X-Gm-Message-State: ALKqPwf1wh5Qg88yKBl1NVUSUuhDtoVHooJRsxdwCpoWFz4vc+Mw44ed mtE/s38dRgmAvsE0QU+C7I0lYA== X-Google-Smtp-Source: AB8JxZrzbCxWdrdu+BmkVywhNNN2yDJE565S4JEU/gxKSrO2MqsrQ7fqKw4wfqQt72p5H+q7jd/0gQ== X-Received: by 2002:a9d:40ad:: with SMTP id n42-v6mr4950208ote.389.1526075734386; Fri, 11 May 2018 14:55:34 -0700 (PDT) Received: from jigglypuff-PC.aus.lan ([64.157.241.12]) by smtp.gmail.com with ESMTPSA id n50-v6sm2481794otb.14.2018.05.11.14.55.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 11 May 2018 14:55:33 -0700 (PDT) From: Andrew D'Addesio <modchipv12@gmail.com> To: gdb-patches@sourceware.org Cc: Andrew D'Addesio <modchipv12@gmail.com> Subject: [PATCH] Process record: Fix null deref when loading empty core file Date: Fri, 11 May 2018 16:54:58 -0500 Message-Id: <1526075698-20880-1-git-send-email-modchipv12@gmail.com> |
Commit Message
Andrew D'Addesio
May 11, 2018, 9:54 p.m. UTC
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(-)
Comments
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
Hi Andrew, The patch you sent seems reasonable to me, and I'd be ready to approve it. But the information you sent below is extremely useful for understanding the conditions necessary to reproduce the issue you are fixing. This is both useful for understanding and reviewing your patch, but also for others who might either find this thread, or else be looking at your patch, and wondering what you were trying to do, when you made your change. With that in mind, can you create a commit which contains the explanation you just gave in the revision log, followed by whatever explanation you feel is necessary to provide as to how you fixed the problem, followed by the ChangeLog entry and a note of how the fix was tested (which platform, which tests that were a FAIL now became a PASS). Browsing the gdb-patches mailing-list for example of how others submit patches should give you a fair idea of what we prefer. Do not hesitate to Cc: me on the resubmission of this specific patch, and I will make every effort to review promptly. Thank you! On Fri, May 11, 2018 at 05:00:29PM -0500, Andrew D'Addesio wrote: > Here's an explanation of the bug: > > Description: [snip]
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)