diff mbox series

[1/2] gdb: fix possible nullptr dereference in a remote_debug_printf call

Message ID 5476235cc65b171f32663fa5e0af0a62342d1f63.1689690655.git.aburgess@redhat.com
State New
Headers
Series Exit during detach |

Commit Message

Andrew Burgess July 18, 2023, 2:31 p.m. UTC 
  While working on the next patch I triggered a segfault from within the
function remote_target::discard_pending_stop_replies.  Turns out this
was caused by a cut&paste error introduced in this commit:

  commit df5ad102009c41ab4dfadbb8cfb8c8b2a02a4f78
  Date:   Wed Dec 1 09:40:03 2021 -0500

      gdb, gdbserver: detach fork child when detaching from fork parent

This commit adds a remote_debug_printf call that was copied from
earlier in the function, however, the new call wasn't updated to use
the appropriate local variable.  The local variable that it is using
might be nullptr, in which case we trigger undefined behaviour, and
could crash, which is what I was seeing.

Fixed by updating to use the correct local variable.
---
 gdb/remote.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Tom Tromey July 19, 2023, 3:01 p.m. UTC | #1 
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> While working on the next patch I triggered a segfault from within the
Andrew> function remote_target::discard_pending_stop_replies.  Turns out this
Andrew> was caused by a cut&paste error introduced in this commit:

Andrew>   commit df5ad102009c41ab4dfadbb8cfb8c8b2a02a4f78
Andrew>   Date:   Wed Dec 1 09:40:03 2021 -0500

Andrew>       gdb, gdbserver: detach fork child when detaching from fork parent

Andrew> This commit adds a remote_debug_printf call that was copied from
Andrew> earlier in the function, however, the new call wasn't updated to use
Andrew> the appropriate local variable.  The local variable that it is using
Andrew> might be nullptr, in which case we trigger undefined behaviour, and
Andrew> could crash, which is what I was seeing.

Andrew> Fixed by updating to use the correct local variable.

Seems obvious to me FWIW.

Tom
Andrew Burgess Aug. 3, 2023, 9:07 a.m. UTC | #2 
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> While working on the next patch I triggered a segfault from within the
> Andrew> function remote_target::discard_pending_stop_replies.  Turns out this
> Andrew> was caused by a cut&paste error introduced in this commit:
>
> Andrew>   commit df5ad102009c41ab4dfadbb8cfb8c8b2a02a4f78
> Andrew>   Date:   Wed Dec 1 09:40:03 2021 -0500
>
> Andrew>       gdb, gdbserver: detach fork child when detaching from fork parent
>
> Andrew> This commit adds a remote_debug_printf call that was copied from
> Andrew> earlier in the function, however, the new call wasn't updated to use
> Andrew> the appropriate local variable.  The local variable that it is using
> Andrew> might be nullptr, in which case we trigger undefined behaviour, and
> Andrew> could crash, which is what I was seeing.
>
> Andrew> Fixed by updating to use the correct local variable.
>
> Seems obvious to me FWIW.

I updated the commit message to not mention "the next patch", and pushed
this fix.

Thanks,
Andrew
diff mbox series

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 7e3d6adfe4f..ff3d7e5cd32 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7564,8 +7564,8 @@  remote_target::discard_pending_stop_replies (struct inferior *inf)
   for (auto it = iter; it != rs->stop_reply_queue.end (); ++it)
     remote_debug_printf
       ("discarding queued stop reply: ptid: %s, ws: %s\n",
-       reply->ptid.to_string().c_str(),
-       reply->ws.to_string ().c_str ());
+       (*it)->ptid.to_string().c_str(),
+       (*it)->ws.to_string ().c_str ());
   rs->stop_reply_queue.erase (iter, rs->stop_reply_queue.end ());
 }