Bug 28224 - Use-after-free when calling a GNU ifunc
Summary: Use-after-free when calling a GNU ifunc
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 13.1
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 27683 29941 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-08-12 21:10 UTC by Simon Marchi
Modified: 2023-01-07 12:21 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Tentative patch (1.54 KB, patch)
2022-12-27 07:24 UTC, Tom de Vries
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Marchi 2021-08-12 21:10:33 UTC
This was initially reported by "randomuser" on IRC.  With the test program:

---
#include <string.h>

int main(int argc, char ** argv)
{
        const char * a = "abcd";
        return strlen(argv[0]);
}
---

This crashes GDB:

$ ./gdb -q -nx --data-directory=data-directory -batch a.out -ex start -ex n -ex 'p (int)strlen(a)'

==620033==ERROR: AddressSanitizer: heap-use-after-free on address 0x62100320f270 at pc 0x55c7a9f79c10 bp 0x7ffef4d555b0 sp 0x7ffef4d555a0                                                          
READ of size 8 at 0x62100320f270 thread T0                                                                                                                                                         
    #0 0x55c7a9f79c0f in get_frame_arch(frame_info*) /home/smarchi/src/binutils-gdb/gdb/frame.c:2841                                                                                               
    #1 0x55c7a9f7a084 in get_frame_sp(frame_info*) /home/smarchi/src/binutils-gdb/gdb/frame.c:2929                                                                                                 
    #2 0x55c7aa0d259b in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:845                         
    #3 0x55c7aa0d1e53 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:742                                                            
    #4 0x55c7a9eedb32 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:674                           
    #5 0x55c7a9eee0d8 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, s
td::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:702                                                    
    #6 0x55c7a96ce44c in expr::var_msym_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >,
 std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expop.h:739                                                 
    #7 0x55c7a99bb864 in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2178                                                             
    #8 0x55c7a9eeb4c5 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:101                                                                                         
    #9 0x55c7a9eeb614 in evaluate_expression(expression*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:115                                                                                     
    #10 0x55c7aa4a6b3c in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1312                                                                                            
    #11 0x55c7aa4a6d14 in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1325                                                                                                       
    #12 0x55c7aa4a7884 in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1458                                                                                                         
    #13 0x55c7a9a73f73 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:97                                                                                                    
    #14 0x55c7a9a7f0e1 in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2141                                                                   
    #15 0x55c7aa9f7ef9 in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:674                                                                                           
    #16 0x55c7aa2c630d in catch_command_errors /home/smarchi/src/binutils-gdb/gdb/main.c:523                                                                                                       
    #17 0x55c7aa2c69df in execute_cmdargs /home/smarchi/src/binutils-gdb/gdb/main.c:618                                                                                                            
    #18 0x55c7aa2c9f9d in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:1322                                                                                                           
    #19 0x55c7aa2ca679 in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1343                                                                                                             
    #20 0x55c7aa2ca71a in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1368                                                                                             
    #21 0x55c7a96259d1 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32                                                                                                                         
    #22 0x7f52a85bf0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)                                                                                                              
    #23 0x55c7a96257ad in _start (/home/smarchi/build/binutils-gdb/gdb/gdb+0x5417ad)   

0x62100320f270 is located 368 bytes inside of 4064-byte region [0x62100320f100,0x6210032100e0)                                                                                                     
freed by thread T0 here:                                                                                                                                                                           
    #0 0x7f52a95b57cf in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10d7cf)                                                                                                      
    #1 0x55c7ab16e1fb in rpl_free /home/smarchi/src/binutils-gdb/gnulib/import/free.c:40                                                                                                           
    #2 0x55c7a9f7ad93 in xfree<void> /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/common-utils.h:66                                                                                            
    #3 0x55c7ab120684 in call_freefun /home/smarchi/src/binutils-gdb/libiberty/obstack.c:103
    #4 0x55c7ab1210c3 in _obstack_free /home/smarchi/src/binutils-gdb/libiberty/obstack.c:280
    #5 0x55c7a9f75b2b in reinit_frame_cache() /home/smarchi/src/binutils-gdb/gdb/frame.c:1999
    #6 0x55c7aa66ea21 in regcache_write_pc(regcache*, unsigned long) /home/smarchi/src/binutils-gdb/gdb/regcache.c:1372
    #7 0x55c7aa124729 in proceed(unsigned long, gdb_signal) /home/smarchi/src/binutils-gdb/gdb/infrun.c:3075
    #8 0x55c7aa0d114d in run_inferior_call /home/smarchi/src/binutils-gdb/gdb/infcall.c:611
    #9 0x55c7aa0d4d3c in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:1277
    #10 0x55c7aa0d1e53 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:742
    #11 0x55c7a9ee0a4d in elf_gnu_ifunc_resolve_addr /home/smarchi/src/binutils-gdb/gdb/elfread.c:917
    #12 0x55c7aa0cfb66 in find_function_addr(value*, type**, type**) /home/smarchi/src/binutils-gdb/gdb/infcall.c:284
    #13 0x55c7aa0d2394 in call_function_by_hand_dummy(value*, type*, gdb::array_view<value*>, void (*)(void*, int), void*) /home/smarchi/src/binutils-gdb/gdb/infcall.c:814
    #14 0x55c7aa0d1e53 in call_function_by_hand(value*, type*, gdb::array_view<value*>) /home/smarchi/src/binutils-gdb/gdb/infcall.c:742
    #15 0x55c7a9eedb32 in evaluate_subexp_do_call(expression*, noside, value*, gdb::array_view<value*>, char const*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:674
    #16 0x55c7a9eee0d8 in expr::operation::evaluate_funcall(type*, expression*, noside, char const*, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >, 
std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/eval.c:702
    #17 0x55c7a96ce44c in expr::var_msym_value_operation::evaluate_funcall(type*, expression*, noside, std::__debug::vector<std::unique_ptr<expr::operation, std::default_delete<expr::operation> >
, std::allocator<std::unique_ptr<expr::operation, std::default_delete<expr::operation> > > > const&) /home/smarchi/src/binutils-gdb/gdb/expop.h:739
    #18 0x55c7a99bb864 in expr::funcall_operation::evaluate(type*, expression*, noside) /home/smarchi/src/binutils-gdb/gdb/expop.h:2178
    #19 0x55c7a9eeb4c5 in expression::evaluate(type*, noside) /home/smarchi/src/binutils-gdb/gdb/eval.c:101
    #20 0x55c7a9eeb614 in evaluate_expression(expression*, type*) /home/smarchi/src/binutils-gdb/gdb/eval.c:115
    #21 0x55c7aa4a6b3c in process_print_command_args /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1312
    #22 0x55c7aa4a6d14 in print_command_1 /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1325
    #23 0x55c7aa4a7884 in print_command /home/smarchi/src/binutils-gdb/gdb/printcmd.c:1458
    #24 0x55c7a9a73f73 in do_simple_func /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:97
    #25 0x55c7a9a7f0e1 in cmd_func(cmd_list_element*, char const*, int) /home/smarchi/src/binutils-gdb/gdb/cli/cli-decode.c:2141
    #26 0x55c7aa9f7ef9 in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:674
    #27 0x55c7aa2c630d in catch_command_errors /home/smarchi/src/binutils-gdb/gdb/main.c:523
    #28 0x55c7aa2c69df in execute_cmdargs /home/smarchi/src/binutils-gdb/gdb/main.c:618
    #29 0x55c7aa2c9f9d in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:1322

previously allocated by thread T0 here:                                                                                                                                                            
    #0 0x7f52a95b5bc8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)                                                                                                                  
    #1 0x55c7a976c1de in xmalloc /home/smarchi/src/binutils-gdb/gdb/alloc.c:60
    #2 0x55c7ab12058e in call_chunkfun /home/smarchi/src/binutils-gdb/libiberty/obstack.c:94
    #3 0x55c7ab120741 in _obstack_begin_worker /home/smarchi/src/binutils-gdb/libiberty/obstack.c:141
    #4 0x55c7ab1209f8 in _obstack_begin /home/smarchi/src/binutils-gdb/libiberty/obstack.c:164
    #5 0x55c7a9f75b52 in reinit_frame_cache() /home/smarchi/src/binutils-gdb/gdb/frame.c:2000
    #6 0x55c7aa9caf4d in switch_to_thread(thread_info*) /home/smarchi/src/binutils-gdb/gdb/thread.c:1319
    #7 0x55c7aa9cb158 in scoped_restore_current_thread::restore() /home/smarchi/src/binutils-gdb/gdb/thread.c:1345
    #8 0x55c7aa9cb3ae in scoped_restore_current_thread::~scoped_restore_current_thread() /home/smarchi/src/binutils-gdb/gdb/thread.c:1365
    #9 0x55c7aa134b5c in stop_all_threads() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4893
    #10 0x55c7aa146ed2 in stop_waiting /home/smarchi/src/binutils-gdb/gdb/infrun.c:7937
    #11 0x55c7aa148117 in end_stepping_range /home/smarchi/src/binutils-gdb/gdb/infrun.c:8104
    #12 0x55c7aa141837 in process_event_stop_test /home/smarchi/src/binutils-gdb/gdb/infrun.c:7192
    #13 0x55c7aa13ca5f in handle_signal_stop /home/smarchi/src/binutils-gdb/gdb/infrun.c:6402
    #14 0x55c7aa13830b in handle_inferior_event /home/smarchi/src/binutils-gdb/gdb/infrun.c:5663
    #15 0x55c7aa12c931 in fetch_inferior_event() /home/smarchi/src/binutils-gdb/gdb/infrun.c:4059 
    #16 0x55c7aa0cb8f4 in inferior_event_handler(inferior_event_type) /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:41
    #17 0x55c7aa246b61 in handle_target_event /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4208 
    #18 0x55c7ab08c5d6 in handle_file_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:575
    #19 0x55c7ab08ce17 in gdb_wait_for_event /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:701
    #20 0x55c7ab08ab38 in gdb_do_one_event() /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:212
    #21 0x55c7aa9f6e8f in wait_sync_command_done() /home/smarchi/src/binutils-gdb/gdb/top.c:528
    #22 0x55c7aa9f705c in maybe_wait_sync_command_done(int) /home/smarchi/src/binutils-gdb/gdb/top.c:545
    #23 0x55c7aa9f7f06 in execute_command(char const*, int) /home/smarchi/src/binutils-gdb/gdb/top.c:676
    #24 0x55c7aa2c630d in catch_command_errors /home/smarchi/src/binutils-gdb/gdb/main.c:523
    #25 0x55c7aa2c69df in execute_cmdargs /home/smarchi/src/binutils-gdb/gdb/main.c:618
    #26 0x55c7aa2c9f9d in captured_main_1 /home/smarchi/src/binutils-gdb/gdb/main.c:1322
    #27 0x55c7aa2ca679 in captured_main /home/smarchi/src/binutils-gdb/gdb/main.c:1343
    #28 0x55c7aa2ca71a in gdb_main(captured_main_args*) /home/smarchi/src/binutils-gdb/gdb/main.c:1368
    #29 0x55c7a96259d1 in main /home/smarchi/src/binutils-gdb/gdb/gdb.c:32

If we look above where the memory was freed, we see two nested call_function_by_hand_dummy calls.  That is because the strlen symbol is an ifunc.  It's a function that returns a pointer to the right strlen implementation to use.  So to call the real strlen function, GDB first needs to determine its address, and for that calls the strlen ifunc.  The pointer to the current pointer that it got in the outer call_function_by_hand_dummy call gets invalidated when the inner call_function_by_hand_dummy runs.  A fix is to re-read the current frame after the find_function_addr call.  This gives the same frame as before, but a fresh / non-stale frame_info object.

From c36675ffcd23a6e5988998bd74d5e79d1638fd13 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Thu, 12 Aug 2021 16:07:55 -0400
Subject: [PATCH] fix

Change-Id: Id4e9abe3db1436bb429bdc8136a64f184f7170d5
---
 gdb/infcall.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 40298fb1318..f0bbac347c8 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -813,6 +813,8 @@ call_function_by_hand_dummy (struct value *function,
   type *values_type;
   CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
 
+  frame = get_current_frame ();
+
   if (values_type == NULL)
     values_type = default_return_type;
   if (values_type == NULL)
-- 
2.32.0

Testing-wise, I was surprised that this isn't caught by gdb.base/gnu-ifunc.exp, which is quite extensive.  This is because it does multiple consecutive infcalls, and starts with some invalid ones.  For example, it starts with:

  gdb_test "p gnu_ifunc()" "Too few arguments in function call\\."

This calls the ifunc successfully, and the ifunc result gets cached (elf_gnu_ifunc_record_cache).  But since the number of arguments don't match, the infcall is aborted and we don't reach where the crash would happen.  When the test then does a second infcall with the proper arguments:

  gdb_test "p gnu_ifunc (3)" " = 4"

The inner infcall to resolve the ifunc isn't done, as the value is cached.  So the frame_info pointer in call_function_by_hand_dummy does not get invalidated.  The test can be modified to see the crash like this:

diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
index 4ec529130ce9..4ce5f4e2dbb8 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -246,13 +246,8 @@ proc misc_tests {resolver_attr resolver_debug final_debug} {
     # Test GDB will automatically indirect the call.
 
     if {!$resolver_debug && !$final_debug} {
-       gdb_test "p gnu_ifunc()" \
-           "'${dot}final' has unknown return type; cast the call to its declared return type"
-       gdb_test "p gnu_ifunc (3)" \
-           "'${dot}final' has unknown return type; cast the call to its declared return type"
        gdb_test "p (int) gnu_ifunc (3)" " = 4"
     } else {
-       gdb_test "p gnu_ifunc()" "Too few arguments in function call\\."
        gdb_test "p gnu_ifunc (3)" " = 4"
     }
 
So, the test would need to be modified to have better coverage.  Either by restarting GDB between each inferior call attempt (that would increase the running time of the test, but correctness is more important) or by having a way to clear the ifunc cache.
Comment 1 Tom Tromey 2022-03-06 17:18:25 UTC
+  frame = get_current_frame ();

Jan pointed out ages ago that the gdb design where a frame can
be silently invalidated by some calls -- and so the caller has
to store a frame-id and "reinflate" the frame -- is fragile.

One idea we've discussed is to use C++ objects for these things,
so that when a frame is invalidated, it will reinflate all on its
own, on reference.  Then the callers don't need to be handled
specially.
Comment 2 Hannes Domani 2022-05-08 13:46:05 UTC
Isn't this a duplicate of PR27683?
Comment 3 Simon Marchi 2022-05-08 17:12:05 UTC
(In reply to Hannes Domani from comment #2)
> Isn't this a duplicate of PR27683?

Yeah, I had an impression of deja vu when replying to #27683.  I will mark #27683 duplicate of this one, since this one has a bit more analysis.  But it would be nice to test the reproducer in #27683 when coming up with a fix.
Comment 4 Simon Marchi 2022-05-08 17:12:28 UTC
*** Bug 27683 has been marked as a duplicate of this bug. ***
Comment 5 Tom de Vries 2022-12-26 13:08:33 UTC
*** Bug 29941 has been marked as a duplicate of this bug. ***
Comment 6 Tom de Vries 2022-12-26 13:13:20 UTC
(In reply to Simon Marchi from comment #0)
> @@ -813,6 +813,8 @@ call_function_by_hand_dummy (struct value *function,
>    type *values_type;
>    CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
>  
> +  frame = get_current_frame ();
> +
>    if (values_type == NULL)
>      values_type = default_return_type;
>    if (values_type == NULL)
> -- 

While analyzing a duplicate, I came up with the exact same fix.

Andrew proposed a different one here: https://sourceware.org/bugzilla/show_bug.cgi?id=29941#c3 .
Comment 7 Tom de Vries 2022-12-26 13:18:41 UTC
(In reply to Tom de Vries from comment #6)
> Andrew proposed a different one here:
> https://sourceware.org/bugzilla/show_bug.cgi?id=29941#c3 .

And I can confirm that it fixes the segfault with the example from PR29941 for me.
Comment 8 philippe.waroquiers 2022-12-26 15:15:27 UTC
The patch also fixes the problem for me.
Comment 9 Mark Wielaard 2022-12-26 22:02:50 UTC
Both the "get_current_frame" patch from this bug as the "reinflate" patch from bug #29941 fix the crash I saw on arm64 as described in https://sourceware.org/bugzilla/show_bug.cgi?id=29941#c1
Comment 10 Simon Marchi 2022-12-27 03:26:48 UTC
This entire class of bugs should be fixed by this series: https://inbox.sourceware.org/gdb-patches/20221214033441.499512-1-simon.marchi@polymtl.ca/
Comment 11 Tom de Vries 2022-12-27 05:25:50 UTC
(In reply to Simon Marchi from comment #10)
> This entire class of bugs should be fixed by this series:
> https://inbox.sourceware.org/gdb-patches/20221214033441.499512-1-simon.
> marchi@polymtl.ca/

Perhaps good though to get a backportable fix for gdb 13.
Comment 12 Tom de Vries 2022-12-27 07:24:35 UTC
Created attachment 14538 [details]
Tentative patch
Comment 13 Tom de Vries 2022-12-27 09:56:06 UTC
(In reply to Tom de Vries from comment #12)
> Created attachment 14538 [details]
> Tentative patch

Tested went ok, submitted: https://sourceware.org/pipermail/gdb-patches/2022-December/195112.html .
Comment 14 Tom de Vries 2023-01-03 10:54:59 UTC
Fixed by commits listed in dup PR29941.
Comment 15 Joel Brobecker 2023-01-07 11:07:46 UTC
IIUC, we can close this PR?
Comment 16 Tom de Vries 2023-01-07 12:21:22 UTC
(In reply to Joel Brobecker from comment #15)
> IIUC, we can close this PR?

Yes, I meant to do that, but that seems to have misfired somehow.

Now marking resolved-fixed.