gdb/infcall: Fix freed frame dereferenced in inferior call

Message ID 20240111163826.286722-1-tlloyddavies@undo.io
State New
Headers
Series gdb/infcall: Fix freed frame dereferenced in inferior call |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Toby Lloyd Davies Jan. 11, 2024, 4:38 p.m. UTC
  Between the store to frame and it's use by get_frame_sp (frame) there is
a call to find_function_addr(). If the function is a IFUNC then this can
do another inferior call. This results in the frame cache being
invalidated which frees all frames. Then the frame pointer previously
stored is left dangling and get_frame_sp (frame) may segfault.
---
 gdb/infcall.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Tom Tromey Jan. 11, 2024, 5:26 p.m. UTC | #1
>>>>> "Toby" == Toby Lloyd Davies <tlloyddavies@undo.io> writes:

Toby> Between the store to frame and it's use by get_frame_sp (frame) there is
Toby> a call to find_function_addr(). If the function is a IFUNC then this can
Toby> do another inferior call. This results in the frame cache being
Toby> invalidated which frees all frames. Then the frame pointer previously
Toby> stored is left dangling and get_frame_sp (frame) may segfault.

This sounds like exactly the kind of bug that should have been fixed by
the frame_info_ptr change -- stale frame_info pointer are no longer
possible.

Was this patch made against an older version of gdb?

If it's still a bug, it might be good to have more information about it.
Also a test case would be handy.

thanks,
Tom
  
Toby Lloyd Davies Jan. 11, 2024, 6:19 p.m. UTC | #2
> Was this patch made against an older version of gdb?

Yes. You're right that this is now fixed so this patch is no longer needed.

Thanks,
Toby


On Thu, 11 Jan 2024 at 17:26, Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Toby" == Toby Lloyd Davies <tlloyddavies@undo.io> writes:
>
> Toby> Between the store to frame and it's use by get_frame_sp (frame) there is
> Toby> a call to find_function_addr(). If the function is a IFUNC then this can
> Toby> do another inferior call. This results in the frame cache being
> Toby> invalidated which frees all frames. Then the frame pointer previously
> Toby> stored is left dangling and get_frame_sp (frame) may segfault.
>
> This sounds like exactly the kind of bug that should have been fixed by
> the frame_info_ptr change -- stale frame_info pointer are no longer
> possible.
>
> Was this patch made against an older version of gdb?
>
> If it's still a bug, it might be good to have more information about it.
> Also a test case would be handy.
>
> thanks,
> Tom
  

Patch

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 8252feea074..15dce04c984 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -938,6 +938,7 @@  call_function_by_hand_dummy (struct value *function,
 
   /* Ensure that the initial SP is correctly aligned.  */
   {
+    frame = get_current_frame ();
     CORE_ADDR old_sp = get_frame_sp (frame);
 
     if (gdbarch_frame_align_p (gdbarch))