Re: gpsmon crashes in ncurses code

[Thomas Dickey]

On Mon, Apr 13, 2020 at 02:38:33PM +0200, SZIGETVÁRI János wrote:
> Dear ncurses maintainers,
> 
> I recently encountered a problem where the gpsmon binary from the gpsd
> package crashes with a segmentation fault.
> According to the backtrace, the crash happened in ncurses code. I already
> had a lengthy discussion with gpsd developers, where they reached the
> conclusion that the gpsmon code that was running until the crash was pretty
> straightforward (it was still initializing the display upon startup), and
> did not yet have the possibility to do anything risky.

actually it's past initializing the display since it's in gpsd_multipoll

risky stuff could happen once it's started threads for reading data,
if there were some bug in its lock/unlock (but in a quick read I
don't see that, either).

> The thread on the gpsd mailing list can be found here:
> https://lists.nongnu.org/archive/html/gpsd-dev/2020-04/msg00018.html

yes... they're offering advice on how to pinpoint the problem
because they're not able to reproduce the problem.
 
> A few words about my environment: It's a Raspberry PI 2, running
> Slackware-ARM current. The system has ncurses 6.2 installed.
> During my tests I found that the code (gpsd version 3.17) that was
> originally compiled and working with ncurses 5.9, crashed when compiled
> with ncurses 6.2.
> The OS ncurses package was built with the .Slackbuild build script found in
> this directory:
> ftp://ftp.slackware.no/slackware/slackware64-current/source/l/ncurses/
> It may be important that the build script applies the following patch
> before building:
> ftp://ftp.slackware.no/slackware/slackware64-current/source/l/ncurses/ncurses.mkhashsize.diff.gz
> 
> The backtrace of the crash is the following:
> 
> (gdb) run
> tc://localhost:2947           JSON slave driver>
> (111)
> {"class":"VERSION","release":"3.20.1~dev","rev":"release-3.20-433-ge897c3f63","proto_major":3,"proto_minor":14}
> (256)
> {"class":"DEVICES","devices":[{"class":"DEVICE","path":"/dev/ttyAMA0","driver":"u-blox","subtype":"SW
> 7.03 (45969),HW
> 00040007","activated":"2020-04-11T18:09:36.250Z","flags":1,"native":1,"bps":9600,"parity":"N","stopbits":1,"cycle":1.00,"mincycle":0.25}]}
> (122)
> {"class":"WATCH","enable":true,"json":false,"nmea":false,"raw":2,"scaled":false,"timing":false,"split24":false,"pps":true}
> 
> Program received signal SIGSEGV, Segmentation fault.
>                                                     0x76e4ebd8 in
> waddch_literal (win=0x217ca0, ch=2097184) at
> ../ncurses/./base/lib_addch.c:391
> 391         line->text[x++] = ch;
> (gdb) list
> 386         });
> 387
> 388         /*
> 389          * Single-column characters.
> 390          */
> 391         line->text[x++] = ch;
> 392         /*
> 393          * This label is used only for wide-characters.
> 394          */
> 395         if_WIDEC(
> (gdb) thread apply all bt full
> 
> Thread 1 (Thread 0x76ff7ff0 (LWP 30236)):
> #0  0x76e4ebd8 in waddch_literal (win=0x217ca0, ch=2097184) at
> ../ncurses/./base/lib_addch.c:391
>         x = 2
>         y = 2
>         line = 0x217d08

The automatically-printed values look reasonable,
but the problem happens with line->text.
Printing
        print *line

might show something, e.g., a value for line->text
If
        print line->text[2]

gives an error, that's confirming valgrind's warning.

If you get an error, then there's still the problem of finding
why the pointer's bad.  (If you aren't able to get an error,
valgrind's probably misleading you on the cause)

Also,
        print *win

might show something (see the lib_addch.c source -- negative values
for _cury and/or _curx would be bad, since "line" is updated at line 348).

Seeing this on line 256:
    line = win->_line + y;

then
        print win->_line + y

should match "line".

However, the debugger says that x and y are both 2,
which is legal for the size of the window that's supposedly in use.

> #1  0x76e4ecf0 in waddch_nosync (win=0x217ca0, ch=32) at
> ../ncurses/./base/lib_addch.c:443
>         x = 33
>         y = 0
>         t = 32
>         sp = 0x112430
>         s = 0x76e30f84 <unctrl_blob+96> " "
>         tabsize = 8
> #2  0x76e4effc in _nc_waddch_nosync (win=0x217ca0, c=32) at
> ../ncurses/./base/lib_addch.c:529
> No locals.
> #3  0x76e4f188 in waddnstr (win=0x217ca0, astr=0x207870 " 0", n=1) at
> ../ncurses/./base/lib_addstr.c:70
>         ch = 32
>         str = 0x207871 "0"
>         code = 0
> #4  0x76e60b70 in vw_printw (win=0x217ca0, fmt=0xc4718 "%2d", argp=...) at
> ../ncurses/./base/lib_printw.c:164
>         buf = 0x207870 " 0"
>         code = -1
>         sp = 0x112430
> #5  0x76e60a88 in mvwprintw (win=0x217ca0, y=2, x=1, fmt=0xc4718 "%2d") at
> ../ncurses/./base/lib_printw.c:127
>         argp = {__ap = 0x7eff6f00}
>         code = 0
> #6  0x0002367c in ubx_initialize () at monitor_ubx.c:34
>         i = 0
> #7  0x00014c5c in switch_type (devtype=0xcc1f8 <driver_ubx>) at gpsmon.c:524
>         leftover = 0
>         trial = 0xed30c <monitor_objects+36>
>         newobject = 0xed30c <monitor_objects+36>
> #8  0x00014eb8 in select_packet_monitor (device=0xf4008 <session>) at
> gpsmon.c:570
>         active_type = 0xcc1f8 <driver_ubx>
>         last_type = 19
> #9  0x00015938 in gpsmon_hook (device=0xf4008 <session>,
> changed=19494547352520246) at gpsmon.c:809
>         buf =
> "\214\000\006\000\000\000\000\000\000\000\000\000h\360\377~\003\000\000\000\020\357\377~\350\357\377~le\":true,\"json\":false,\"nmea\":false,\"raw\":2,\"scaled\":false,\"timing\":false,\"split24\":false,\"pps\":true}\n\000\060\067\",\"activated\":\"2020-04-11T18:09:36.250Z\",\"flags\":1,\"native\":1,\"bps\":"...
>         ts_buf1 =
> "\b\000\000\000\240%\f\000\000\000\000\000\070\060\001\000\000\000\000\000\000"
>         ts_buf2 =
> "\340\b\310v\324\316\377~\240%\f\000\000\000\000\000\070\060\001\000\000"
>         __PRETTY_FUNCTION__ = "gpsmon_hook"
> #10 0x00061b7c in gpsd_multipoll (data_ready=true, device=0xf4008
> <session>, handler=0x154e8 <gpsmon_hook>, reawake_time=0) at
> libgpsd_core.c:1844
>         changed = 19494547352520246
>         fragments = 0
> #11 0x00017844 in main (argc=1, argv=0x7efff264) at gpsmon.c:1398
>         efds = {fds_bits = {0 <repeats 32 times>}}
>         option = -1
>         explanation = 0x0
>         bailout = 0
>         matches = 0
>         nmea = false
>         all_fds = {fds_bits = {9, 0 <repeats 31 times>}}
>         rfds = {fds_bits = {8, 0 <repeats 31 times>}}
>         maxfd = 3
>         inbuf =
> "x\365\377v\300\a\310vH\246\377v\000\000\000\000\000\360\377~0y\377v
> \304\374v\205\317c\t\003\000\000\000x\365\377vj*\336v0y\377v(\360\377~\220\302\374v\020n\375v\001\000\000\000
> \304\374v\026\000\000\000\360\304\374vx\365\377v"
>         nocurses = false
>         activated = 3
> (gdb)
> 
> 
> Valgrind's output is the following:
> 
> 
> ==13245== Memcheck, a memory error detector
> ==13245== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==13245== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
> ==13245== Command: gpsmon
> ==13245==
> ==13245== Invalid write of size 4
> ==13245==    at 0x145E4: refresh_statwin (gpsmon.c:441)
> ==13245==    by 0x149F7: curses_init (gpsmon.c:489)
> ==13245==    by 0x176E7: main (gpsmon.c:1378)
> ==13245==  Address 0x4cddbe8 is 24 bytes before a block of size 16 in arena
> "client"

That's the specific error message, which (in context) is saying that
"line->text" has been set to a bogus value, and that adding to it
makes valgrind report this.

But as I commented above, you can investigate that in the debugger.

-- 
Thomas E. Dickey <address@hidden>
https://invisible-island.net
ftp://ftp.invisible-island.net

signature.asc
Description: PGP signature