[Bug breakpoints/29960] New: gdb doesn't prevent user from setting faulty catch condition and segfaults

[Bug breakpoints/29960] New: gdb doesn't prevent user from setting faulty catch condition and segfaults

[festerdam at posteo dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=29960

            Bug ID: 29960
           Summary: gdb doesn't prevent user from setting faulty catch
                    condition and segfaults
           Product: gdb
           Version: HEAD
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: breakpoints
          Assignee: unassigned at sourceware dot org
          Reporter: festerdam at posteo dot net
  Target Milestone: ---

When inputting the following to gdb:

r
# Let child exit normally
catch syscall 1
condition 1 fd == 1 #assume catchpoint obtained above is 1
# Will fail saying that no symbol table is loaded or that the symbol doesn't
# exist. Outcome in the end will be the same
condition 1 write.fd == 1 # won't fail for some reason
r # segfault occurs now

It seems I always have to run the program prior to setting the syscall in order
for this to happen.

I tested it with GNU Hello, GNU Bash and GNU Guile and with gdb version 12.1-4
(obtained from the Debian repositories) and with gdb commit
ce6fcad80eb594228a2e46e9362d6083881fe96d. They all crashed.

This is the stack trace of spat out by gdb ce6fcad80 running GNU Hello:
Fatal signal: Segmentation fault

----- Backtrace -----
0x55555570204b gdb_internal_backtrace_1
        ../../gdb/bt-utils.c:122
0x55555570204b _Z22gdb_internal_backtracev
        ../../gdb/bt-utils.c:168
0x555555813421 handle_fatal_signal
        ../../gdb/event-top.c:956
0x55555581358c handle_sigsegv
        ../../gdb/event-top.c:1029
0x7ffff6e5af8f ???
        ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x555555a35a90 _ZNK19general_symbol_info13section_indexEv
        ../../gdb/symtab.h:605
0x555555a35a90 _ZNK19general_symbol_info11obj_sectionEPK7objfile
        ../../gdb/symtab.c:1090
0x555555952139 _Z28find_minsym_type_and_addressP14minimal_symbolP7objfilePm
        ../../gdb/parse.c:105
0x55555580e373 _Z23evaluate_var_msym_value6nosideP7objfileP14minimal_symbol
        ../../gdb/eval.c:597
0x55555580e89e
_Z22eval_op_var_msym_valueP4typeP10expression6nosideb20bound_minimal_symbol
        ../../gdb/eval.c:1113
0x555555716e10 _ZN4expr18structop_operation8evaluateEP4typeP10expression6noside
        ../../gdb/expop.h:1034
0x555555716d14
_ZN4expr20comparison_operationIL10exp_opcode14EXadL_Z13eval_op_equalP4typeP10expression6nosideS1_P5valueS8_EEE8evaluateES3_S5_S6_
        ../../gdb/expop.h:1334
0x55555580d55c _ZN10expression8evaluateEP4type6noside
        ../../gdb/eval.c:101
0x5555556eddb1 breakpoint_cond_eval
        ../../gdb/breakpoint.c:4971
0x5555556eddb1 bpstat_check_breakpoint_conditions
        ../../gdb/breakpoint.c:5540
0x5555556f6616
_Z18bpstat_stop_statusPK13address_spacemP11thread_infoRK17target_waitstatusP6bpstat
        ../../gdb/breakpoint.c:5721
0x5555558b5985 handle_syscall_event
        ../../gdb/infrun.c:4625
0x5555558b5eb7 handle_inferior_event
        ../../gdb/infrun.c:5832
0x5555558b82fe _Z20fetch_inferior_eventv
        ../../gdb/infrun.c:4202
0x555555c286d5 gdb_wait_for_event
        ../../gdbsupport/event-loop.cc:716
0x555555c291b2 _Z16gdb_do_one_eventi
        ../../gdbsupport/event-loop.cc:264
0x5555558fb609 start_event_loop
        ../../gdb/main.c:411
0x5555558fb609 captured_command_loop
        ../../gdb/main.c:471
0x5555558fd164 captured_main
        ../../gdb/main.c:1310
0x5555558fd164 _Z8gdb_mainP18captured_main_args
        ../../gdb/main.c:1325
0x5555556614a9 main
        ../../gdb/gdb.c:32


And this is the stack trace I get when I run gdb with gdb (not sure why they
are different):

#0  general_symbol_info::obj_section (this=this@entry=0x7fffee85f4e0, 
    objfile=objfile@entry=0x5555561fdf80) at ../../gdb/symtab.c:1090
#1  0x000055555595213a in find_minsym_type_and_address (msymbol=0x7fffee85f4e0, 
    objfile=0x5555561fdf80, address_p=address_p@entry=0x7fffffffd5d8) at
../../gdb/parse.c:105
#2  0x000055555580e374 in evaluate_var_msym_value
(noside=noside@entry=EVAL_NORMAL, 
    objfile=<optimized out>, msymbol=<optimized out>) at ../../gdb/eval.c:597
#3  0x000055555580e89f in eval_op_var_msym_value (expect_type=<optimized out>,
exp=<optimized out>, 
    noside=EVAL_NORMAL, outermost_p=<optimized out>, msymbol=...) at
../../gdb/eval.c:1113
#4  0x0000555555716e11 in expr::structop_operation::evaluate
(this=0x55555626d2c0, expect_type=0x0, 
    exp=0x55555623fa20, noside=EVAL_NORMAL) at ../../gdb/expop.h:1034
#5  0x0000555555716d15 in expr::comparison_operation<(exp_opcode)14,
&(eval_op_equal(type*, expression*, noside, exp_opcode, value*,
value*))>::evaluate (this=0x55555623fa40, expect_type=0x0, 
    exp=0x55555623fa20, noside=EVAL_NORMAL) at ../../gdb/expop.h:1334
#6  0x000055555580d55d in expression::evaluate (this=0x55555623fa20,
expect_type=0x0, 
    noside=EVAL_NORMAL) at ../../gdb/eval.c:101
#7  0x00005555556eddb2 in breakpoint_cond_eval (exp=0x55555623fa20) at
../../gdb/breakpoint.c:4971
#8  bpstat_check_breakpoint_conditions (bs=<optimized out>,
thread=0x5555561b8800)
    at ../../gdb/breakpoint.c:5540
#9  0x00005555556f6617 in bpstat_stop_status (aspace=<optimized out>,
bp_addr=<optimized out>, 
    thread=0x5555561b8800, ws=..., stop_chain=<optimized out>) at
../../gdb/breakpoint.c:5721
#10 0x00005555558b5986 in handle_syscall_event (ecs=ecs@entry=0x7fffffffdd80)
    at ../../gdb/regcache.h:344
#11 0x00005555558b5eb8 in handle_inferior_event (ecs=0x7fffffffdd80) at
../../gdb/infrun.c:5832
--Type <RET> for more, q to quit, c to continue without paging--c
#12 0x00005555558b82ff in fetch_inferior_event () at ../../gdb/infrun.c:4202
#13 0x0000555555c286d6 in gdb_wait_for_event (block=block@entry=1) at
../../gdbsupport/event-loop.cc:716
#14 0x0000555555c291b3 in gdb_do_one_event (mstimeout=mstimeout@entry=-1) at
../../gdbsupport/event-loop.cc:264
#15 0x00005555558fb60a in start_event_loop () at ../../gdb/main.c:411
#16 captured_command_loop () at ../../gdb/main.c:471
#17 0x00005555558fd165 in captured_main (data=data@entry=0x7fffffffded0) at
../../gdb/main.c:1310
#18 gdb_main (args=args@entry=0x7fffffffdf00) at ../../gdb/main.c:1325
#19 0x00005555556614aa in main (argc=<optimized out>, argv=<optimized out>) at
../../gdb/gdb.c:32

I have a coredump, if needed. It was too big to attach.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/29960] gdb doesn't prevent user from setting faulty catch condition and segfaults

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29960

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |tromey at sourceware dot org
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-01-04

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
Was able to reproduce.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/29960] gdb doesn't prevent user from setting faulty catch condition and segfaults

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29960

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
I think the problem is that re-running does not end up reparsing
the breakpoint condition on the catch.  i.e., a bug in the
breakpoint refactoring.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/29960] gdb doesn't prevent user from setting faulty catch condition and segfaults

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29960

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
I was able to reproduce this as far back as 835e063d3a0, 
which is before the breakpoint changes I was thinking of.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/29960] gdb doesn't prevent user from setting faulty catch condition and segfaults

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29960

--- Comment #4 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a92e943014f5e8d6a2eaccaf8a725941ac47a121

commit a92e943014f5e8d6a2eaccaf8a725941ac47a121
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Aug 14 15:16:46 2024 +0100

    gdb: implement ::re_set method for catchpoint class

    It is possible to attach a condition to a catchpoint.  This can't be
    done when the catchpoint is created, but can be done with the
    'condition' command, this is documented in the GDB manual:

         You can also use the 'if' keyword with the 'watch' command.  The
      'catch' command does not recognize the 'if' keyword; 'condition' is the
      only way to impose a further condition on a catchpoint.

    A GDB crash was reported against Fedora GDB where a user had attached
    a condition to a catchpoint and then restarted the inferior.  When the
    catchpoint was hit GDB would immediately segfault.  I was able to
    reproduce the failure on upstream GDB:

      (gdb) file ./some/binary
      (gdb) catch syscall write
      (gdb) run
      ...
      Catchpoint 1 (returned from syscall write), 0x00007ffff7b594a7 in write
() from /lib64/libc.so.6
      (gdb) condition 1 $_streq((char *) $rsi, "foobar") == 0
      (gdb) run
      ...
      Fatal signal: Segmentation fault
      ...

    What happened here is that on the system in question we had debug
    information available for both the main application and also for
    libc.

    When the condition was attached GDB was stopped inside libc and as the
    debug information was available GDB found a reference to the 'char'
    type (for the cast) inside libc's debug information.

    When the inferior is restarted GDB discards all of the objfiles
    associated with shared libraries, and this includes libc.  As such the
    'char' type, which is objfile owned, is discarded and the reference to
    it from the catchpoint's condition expression becomes invalid.

    Now, if it were a breakpoint instead of a catchpoint, what would
    happen is that after the shared library objfiles had been discarded
    we'd call the virtual breakpoint::re_set method on the breakpoint, and
    this would update the breakpoint's condition expression.  This is
    because user breakpoints are actually instances of the code_breakpoint
    class and the code_breakpoint::re_set method contains the code to
    recompute the breakpoint's condition expression.

    However, catchpoints are instances of the catchpoint class which
    inherits from the base breakpoint class.  The catchpoint class does
    not override breakpoint::re_set, and breakpoint::re_set is empty!

    The consequence of this is that catchpoint condition expressions are
    never recomputed, and the dangling pointer to the now deleted, objfile
    owned type 'char' is left around, and, when the catchpoint is hit, the
    invalid pointer is used when GDB tries to evaluate the condition
    expression.

    In this commit I have implemented catchpoint::re_set.  This is pretty
    simple and just recomputes the condition expression as you'd expect.
    If the condition doesn't evaluate then the catchpoint is marked as
    disabled_by_cond.

    I have also made breakpoint::re_set pure virtual.  With the addition
    of catchpoint::re_set every sub-class of breakpoint now implements the
    ::re_set method, and if new sub-classes are added in the future I
    think that they _must_ implement ::re_set in order to avoid this
    problem.  As such falling back to an empty breakpoint::re_set doesn't
    seem helpful.

    For testing I have not relied on stopping in libc and having libc
    debug information available, this doesn't seem like a good idea for
    the GDB testsuite.  Instead I create a (rather pointless) condition
    check that uses a type defined only within a shared library.  When the
    inferior is restarted the catchpoint will temporarily be marked as
    disabled_by_cond (due to the type not being available), but once the
    shared library is loaded again the catchpoint will be re-enabled.
    Without the fixes above then the same crashing behaviour can be
    observed.

    One point of note: the dangling pointer of course exposes undefined
    behaviour, with no guarantee of a crash.  Though a crash is what I
    usually see I have see GDB throw random errors from the expression
    evaluation code, and once, I saw no problem at all!  If you recompile
    GDB with the address sanitizer, or run under valgrind, then the bug
    will be exposed every time.

    After fixing this bug I checked bugzilla and found PR gdb/29960 which
    is the same bug.  I was able to reproduce the bug before this commit,
    and after this commit GDB is no longer crashing.

    Before:

      (gdb) file /tmp/hello.x
      Reading symbols from /tmp/hello.x...
      (gdb) run
      Starting program: /tmp/hello.x
      Hello World
      [Inferior 1 (process 1101855) exited normally]
      (gdb) catch syscall 1
      Catchpoint 1 (syscall 'write' [1])
      (gdb) condition 1 write.fd == 1
      (gdb) run
      Starting program: /tmp/hello.x

      Fatal signal: Segmentation fault
      ...

    And after:

      (gdb) file /tmp/hello.x
      Reading symbols from /tmp/hello.x...
      (gdb) run
      Starting program: /tmp/hello.x
      Hello World
      Args: ( 0 , 1 , 2 , 3 , 4 , 5 , 6 , 7 )
      [Inferior 1 (process 1102373) exited normally]
      (gdb) catch syscall 1
      Catchpoint 1 (syscall 'write' [1])
      (gdb) condition 1 write.fd == 1
      (gdb) r
      Starting program: /tmp/hello.x
      Error in testing condition for breakpoint 1:
      Attempt to extract a component of a value that is not a structure.

      Catchpoint 1 (call to syscall write), 0x00007ffff7eb94a7 in write ()
         from /lib64/libc.so.6
      (gdb) ptype write
      type = <unknown return type> ()
      (gdb)

    Notice we get the error now when the condition fails to evaluate.
    This seems reasonable given that 'write' will be a function, and
    indeed the final 'ptype' shows that it's a function, not a struct.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29960

    Reviewed-By: Tom de Vries <tdevries@suse.de>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/29960] gdb doesn't prevent user from setting faulty catch condition and segfaults

[aburgess at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29960

Andrew Burgess <aburgess at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
                 CC|                            |aburgess at redhat dot com
             Status|NEW                         |RESOLVED

--- Comment #5 from Andrew Burgess <aburgess at redhat dot com> ---
I think this should now be fixed.  If not then feel free to reopen.

-- 
You are receiving this mail because:
You are on the CC list for the bug.