From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2BC1B3858D33 for ; Tue, 6 Jun 2023 15:50:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2BC1B3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686066610; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=jNDP+a7kJ5FDrNrP9sSbsjGJSQVrrUdQuBp2veo9Nrc=; b=XLi4i79sZ4ihF4SYYTpAjauTIlECXSh0RVCziLssZIPGhqBuq7MU6Xrz44Eu7slCFYtvKU IJo4J68kqMMidZj57Ghd1bBUFKDFWvblet3N6lvvIu04aBDKgGTctq0p5jp+/HQ/F0h6xm Oy+aBR5A92MBZR7MnBwda93hDk/mvCA= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-549-D9xfj_IJPQWJU-Z4sDfukQ-1; Tue, 06 Jun 2023 11:50:09 -0400 X-MC-Unique: D9xfj_IJPQWJU-Z4sDfukQ-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-3f5fc8581a9so32167755e9.0 for ; Tue, 06 Jun 2023 08:50:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686066608; x=1688658608; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=jNDP+a7kJ5FDrNrP9sSbsjGJSQVrrUdQuBp2veo9Nrc=; b=cTfdPPuqEGQ7sIEjaxjxlzAqv2oYUisqVe4A5jxHyEqtGo8zJmkuwMZH692k0thwZp AVum1QmZQA4LyydlRhkq7tjENexgTrl46FUXBWdg0IK3cM8268rmfNeSpFTS6fi5fA7n 5SVsT0e6ynoCbOeetpWhL46Phf+54EghiOB67/CLcqTCv1QyRXMMMmJ8nt9MP1vNco/0 sDbYAxZ4/tR6wEE4ETzvS6Vv7AuFMiMBf6ViJH1sVnyZPKYs8kujXeJzvRrwRLA7VVFO Ks8euYfCNte6GRmGRcQlcNvWjWvEtrJnA7mbzzQrDnkHi0bh1OCtYKdesjhc+9zHwCRl r3Gg== X-Gm-Message-State: AC+VfDwcCnyPV82p9u/Gx01F61kUXBYD2Fp6A4xOqeU1Ve1pmenFvU1A VNGma2Kox6CfrGn83+Y7OMQS5kHhTQIR4sYcUWXTpilrd3KTpfHLXVDqcqpLKoUQ515C3GIAiL5 5m22Klv92n72oMdHtlxbLNoRMqCM1Aw== X-Received: by 2002:a05:600c:205a:b0:3f7:536e:fff3 with SMTP id p26-20020a05600c205a00b003f7536efff3mr2516307wmg.25.1686066607773; Tue, 06 Jun 2023 08:50:07 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4jHzfKUK3zWjOBjSdyZ05u9J3szyTs2XaLAwMZtz8c+HeH8PUSXB5vrHtSg9Qbb+VIyJHupg== X-Received: by 2002:a05:600c:205a:b0:3f7:536e:fff3 with SMTP id p26-20020a05600c205a00b003f7536efff3mr2516293wmg.25.1686066607366; Tue, 06 Jun 2023 08:50:07 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id m15-20020a7bca4f000000b003f733c1129fsm10950601wml.33.2023.06.06.08.50.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Jun 2023 08:50:06 -0700 (PDT) From: Andrew Burgess To: Simon Marchi , gdb-patches@sourceware.org Cc: Pedro Alves , Simon Marchi Subject: Re: [PATCHv3] gdb: building inferior strings from within GDB In-Reply-To: References: <75628df1fbd72166504c9b2a368170121b86e072.1680849242.git.aburgess@redhat.com> <878rcy3xj8.fsf@redhat.com> Date: Tue, 06 Jun 2023 16:50:05 +0100 Message-ID: <87edmo380i.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_ASCII_DIVIDERS,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Simon Marchi writes: > On 6/5/23 08:26, Andrew Burgess via Gdb-patches wrote: >> You are right. I merged these two calls, and the other two in >> str_value_from_setting, and pushed this patch. > > Turns out this test triggers an ASan error: > > (gdb) PASS: gdb.base/internal-string-values.exp: test_setting: all langs: lang=ada: ptype "foo" > print $_gdb_maint_setting("test-settings string") > ================================================================= > ==80377==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000068034 at pc 0x564785cba682 bp 0x7ffd20644620 sp 0x7ffd20644610 > READ of size 1 at 0x603000068034 thread T0 Thanks for pointing this out. I see what's going on here. What about this patch for fixing it? Thanks, Andrew --- commit 2cf56b25deb3e74de60cf165f3f852a871125136 Author: Andrew Burgess Date: Tue Jun 6 16:34:35 2023 +0100 gdb: fix ASan failure after recent string changes After this commit: commit baab375361c365afee2577c94cbbd3fdd443d6da Date: Tue Jul 13 14:44:27 2021 -0400 gdb: building inferior strings from within GDB It was pointed out that a new ASan failure had been introduced which was triggered by gdb.base/internal-string-values.exp: (gdb) PASS: gdb.base/internal-string-values.exp: test_setting: all langs: lang=ada: ptype "foo" print $_gdb_maint_setting("test-settings string") ================================================================= ==80377==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000068034 at pc 0x564785cba682 bp 0x7ffd20644620 sp 0x7ffd20644610 READ of size 1 at 0x603000068034 thread T0 #0 0x564785cba681 in find_command_name_length(char const*) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2129 #1 0x564785cbacb2 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string, std::allocator >*, int, bool) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2186 #2 0x564785cbb539 in lookup_cmd_1(char const**, cmd_list_element*, cmd_list_element**, std::__cxx11::basic_string, std::allocator >*, int, bool) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2248 #3 0x564785cbbcf3 in lookup_cmd(char const**, cmd_list_element*, char const*, std::__cxx11::basic_string, std::allocator >*, int, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2339 #4 0x564785c82df2 in setting_cmd /tmp/src/binutils-gdb/gdb/cli/cli-cmds.c:2219 #5 0x564785c84274 in gdb_maint_setting_internal_fn /tmp/src/binutils-gdb/gdb/cli/cli-cmds.c:2348 #6 0x564788167b3b in call_internal_function(gdbarch*, language_defn const*, value*, int, value**) /tmp/src/binutils-gdb/gdb/value.c:2321 #7 0x5647854b6ebd in expr::ada_funcall_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:11254 #8 0x564786658266 in expression::evaluate(type*, noside) /tmp/src/binutils-gdb/gdb/eval.c:111 #9 0x5647871242d6 in process_print_command_args /tmp/src/binutils-gdb/gdb/printcmd.c:1322 #10 0x5647871244b3 in print_command_1 /tmp/src/binutils-gdb/gdb/printcmd.c:1335 #11 0x564787125384 in print_command /tmp/src/binutils-gdb/gdb/printcmd.c:1468 #12 0x564785caac44 in do_simple_func /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:95 #13 0x564785cc18f0 in cmd_func(cmd_list_element*, char const*, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2735 #14 0x564787c70c68 in execute_command(char const*, int) /tmp/src/binutils-gdb/gdb/top.c:574 #15 0x564786686180 in command_handler(char const*) /tmp/src/binutils-gdb/gdb/event-top.c:543 #16 0x56478668752f in command_line_handler(std::unique_ptr >&&) /tmp/src/binutils-gdb/gdb/event-top.c:779 #17 0x564787dcb29a in tui_command_line_handler /tmp/src/binutils-gdb/gdb/tui/tui-interp.c:104 #18 0x56478668443d in gdb_rl_callback_handler /tmp/src/binutils-gdb/gdb/event-top.c:250 #19 0x7f4efd506246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb) #20 0x564786683dea in gdb_rl_callback_read_char_wrapper_noexcept /tmp/src/binutils-gdb/gdb/event-top.c:192 #21 0x564786684042 in gdb_rl_callback_read_char_wrapper /tmp/src/binutils-gdb/gdb/event-top.c:225 #22 0x564787f1b119 in stdin_event_handler /tmp/src/binutils-gdb/gdb/ui.c:155 #23 0x56478862438d in handle_file_event /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:573 #24 0x564788624d23 in gdb_wait_for_event /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:694 #25 0x56478862297c in gdb_do_one_event(int) /tmp/src/binutils-gdb/gdbsupport/event-loop.cc:264 #26 0x564786df99f0 in start_event_loop /tmp/src/binutils-gdb/gdb/main.c:412 #27 0x564786dfa069 in captured_command_loop /tmp/src/binutils-gdb/gdb/main.c:476 #28 0x564786dff61f in captured_main /tmp/src/binutils-gdb/gdb/main.c:1320 #29 0x564786dff75c in gdb_main(captured_main_args*) /tmp/src/binutils-gdb/gdb/main.c:1339 #30 0x564785381b6d in main /tmp/src/binutils-gdb/gdb/gdb.c:32 #31 0x7f4efbc3984f (/usr/lib/libc.so.6+0x2384f) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e) #32 0x7f4efbc39909 in __libc_start_main (/usr/lib/libc.so.6+0x23909) (BuildId: 2f005a79cd1a8e385972f5a102f16adba414d75e) #33 0x564785381934 in _start (/tmp/build/binutils-gdb/gdb/gdb+0xabc5934) (BuildId: 90de353ac158646e7dab501b76a18a76628fca33) 0x603000068034 is located 0 bytes after 20-byte region [0x603000068020,0x603000068034) allocated by thread T0 here: #0 0x7f4efcee0cd1 in __interceptor_calloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x5647856265d8 in xcalloc /tmp/src/binutils-gdb/gdb/alloc.c:97 #2 0x564788610c6b in xzalloc(unsigned long) /tmp/src/binutils-gdb/gdbsupport/common-utils.cc:29 #3 0x56478815721a in value::allocate_contents(bool) /tmp/src/binutils-gdb/gdb/value.c:929 #4 0x564788157285 in value::allocate(type*, bool) /tmp/src/binutils-gdb/gdb/value.c:941 #5 0x56478815733a in value::allocate(type*) /tmp/src/binutils-gdb/gdb/value.c:951 #6 0x5647854ae81c in expr::ada_string_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:10675 #7 0x5647854b63b8 in expr::ada_funcall_operation::evaluate(type*, expression*, noside) /tmp/src/binutils-gdb/gdb/ada-lang.c:11184 #8 0x564786658266 in expression::evaluate(type*, noside) /tmp/src/binutils-gdb/gdb/eval.c:111 #9 0x5647871242d6 in process_print_command_args /tmp/src/binutils-gdb/gdb/printcmd.c:1322 #10 0x5647871244b3 in print_command_1 /tmp/src/binutils-gdb/gdb/printcmd.c:1335 #11 0x564787125384 in print_command /tmp/src/binutils-gdb/gdb/printcmd.c:1468 #12 0x564785caac44 in do_simple_func /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:95 #13 0x564785cc18f0 in cmd_func(cmd_list_element*, char const*, int) /tmp/src/binutils-gdb/gdb/cli/cli-decode.c:2735 #14 0x564787c70c68 in execute_command(char const*, int) /tmp/src/binutils-gdb/gdb/top.c:574 #15 0x564786686180 in command_handler(char const*) /tmp/src/binutils-gdb/gdb/event-top.c:543 #16 0x56478668752f in command_line_handler(std::unique_ptr >&&) /tmp/src/binutils-gdb/gdb/event-top.c:779 #17 0x564787dcb29a in tui_command_line_handler /tmp/src/binutils-gdb/gdb/tui/tui-interp.c:104 #18 0x56478668443d in gdb_rl_callback_handler /tmp/src/binutils-gdb/gdb/event-top.c:250 #19 0x7f4efd506246 in rl_callback_read_char (/usr/lib/libreadline.so.8+0x3b246) (BuildId: 092e91fc4361b0ef94561e3ae03a75f69398acbb) The problem is in cli/cli-cmds.c, in the function setting_cmd, where we do this: const char *a0 = (const char *) argv[0]->contents ().data (); Here argv[0] is a value* which we know is either a TYPE_CODE_ARRAY or a TYPE_CODE_STRING. The problem is that the above line is casting the value contents directly to a C-string, i.e. one that is assumed to have a null-terminator at the end. After the above commit this can no longer be assumed to be true. A string value will be represented just as it would be in the current language, so for Ada and Fortran the string will be an array of characters with no null-terminator at the end. My proposed solution is to copy the string contents into a std::string object, and then use the std::string::c_str() value, this will ensure that a null-terminator has been added. I had a check through GDB at places TYPE_CODE_STRING was used and couldn't see any other obvious places where this type of assumption was being made, so hopefully this is the only offender. Running the above test with ASan compiled in no longer gives an error. diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 8994cc5ea3d..638c138e7cb 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -2215,7 +2215,13 @@ setting_cmd (const char *fnname, struct cmd_list_element *showlist, && type0->code () != TYPE_CODE_STRING) error (_("First argument of %s must be a string."), fnname); - const char *a0 = (const char *) argv[0]->contents ().data (); + /* Not all languages null-terminate their strings, by moving the string + content into a std::string we ensure that a null-terminator is added. + For languages that do add a null-terminator the std::string might end + up with two null characters at the end, but that's harmless. */ + const std::string setting ((const char *) argv[0]->contents ().data (), + type0->length ()); + const char *a0 = setting.c_str (); cmd_list_element *cmd = lookup_cmd (&a0, showlist, "", NULL, -1, 0); if (cmd == nullptr || cmd->type != show_cmd)