Re: [Bug report] coredump when ncurses allocated fail

[Thomas Dickey]

On Mon, Jun 19, 2023 at 02:49:34PM -0400, Thomas Dickey wrote:
> On Mon, Jun 19, 2023 at 09:45:18PM +0800, eaglegai wrote:
> > hi，
> > I'm doing some tests about ncurses, and find some problems.
> 
> This is missing some information:
> 
> a) what version (and patch-date)?
> 
> b) there's no sample program provided to demonstrate the problems

Your report appears to say that it's happening in slabtop (part of procps).

In a quick read/review of its source, I don't see anything that would be
related to the recent changes to tparm/tiparm/tiparm_s, so a failure due
to running out of memory seems to be what you're describing.  That doesn't
happen often enough to get much attention.

I suppose I could reproduce this by setting the result from malloc/realloc
to null, to simulate a failure.  But in a real out-of-memory situation,
there's going to be very little that the caller can do to report the problem.
It can only use already-allocated memory.

For instance, if fmt_buff is null, there's nothing useful that can be done,
and ncurses _could_ simply report a fatal error, but the expected fix would be
to simply return a null from tparm, etc.  Trying to work around this
with isolated pointer checks only hides the problem.

> > there are two solutions which will fix the crash.
> > solutions1:  check oldhash and newhash to avoid using random shift , patch: 
> > solutions1-for-problems2.patch
> > solutions2:  change void  _nc_hash_map to int  _nc_hash_map to avoid using 
> > random shift , patch: solutions2-for-problems2.patch
> > 3. problems 3
> > myname malloc failure, and then strlen(myname) causes coredump.
> > bt:
> > Core was generated by `/usr/bin/slabtop -d 5'.
> > Program terminated with signal SIGSEGV, Segmentation fault.
> > #0 __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:78
> > 78 VPCMP $0, (%rdi), %YMMZERO, %k0
> > (gdb) bt
> > #0 __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:78

Agreed, I should always check the return value of strdup, but overlooked it
in a few cases where it's supposedly known to be limited in size - and
in that case, a malloc failure indicates worse problems...

If you're actually running out of memory, the suggested fix for hashmap
can be helpful, but there are other places which may need attention.

There are about a hundred places to check, when one counts malloc, calloc
and realloc calls - but by redirecting the calls to some debugging feature
that would stop new allocations after some preset value (e.g., using
LD_PRELOAD), I could work through some scenarios of initialization,
to get better coverage.  The existing tools that I see (e.g., manipulating
ulimit, etc.) are all too crude to be useful for this.

> > #1 0x00007fda0d1585f3 in _nc_setupterm (tname=0x55770b37d3b0 "xterm", 
> > Filedes=1, errret=0x7ffe562a1b44, reuse=0) at 
> > ../../ncurses/tinfo/lib_setup.c:683

now... a failure in setupterm/newterm indicates that slabtop was started
when there's not enough memory to run.  The fmt_buff problem _could_ be
after it's running a while.  Since slabtop doesn't check return values on
any of the curses calls, it'll just _sit_ there and go nowhere when it runs
out of memory.  But sure, with some work I can keep ncurses from crashing,
even if it's not able to do anything.

> > #2 0x00007fda0d18f121 in newterm_sp (sp=0x55770b37da10, name=0x55770b37d3b0 
> > "xterm", ofp=0x7fda0d1335a0 <_IO_2_1_stdout_>, ifp=0x7fda0d1328c0 
> > <_IO_2_1_stdin_>) at ../../ncurses/base/lib_newterm.c:212
> > #3 0x00007fda0d18f5ff in newterm (name=0x55770b37d3b0 "xterm", 
> > ofp=0x7fda0d1335a0 <_IO_2_1_stdout_>, ifp=0x7fda0d1328c0 <_IO_2_1_stdin_>) 
> > at ../../ncurses/base/lib_newterm.c:367
> > #4 0x00007fda0d189aa1 in initscr () at ../../ncurses/base/lib_initscr.c:93
> > #5 0x00005577094526dc in main (argc=3, argv=<optimized out>) at 
> > slabtop.c:317
> > (gdb) f 1
> > #1 0x00007fda0d1585f3 in _nc_setupterm (tname=0x55770b37d3b0 "xterm", 
> > Filedes=1, errret=0x7ffe562a1b44, reuse=0) at 
> > ../../ncurses/tinfo/lib_setup.c:683
> > 683 if (strlen(myname) > MAX_NAME_SIZE) {
> > (gdb) l 680
> > 675 if (!NonEmpty(tname)) {
> > 676 T(("Failure with TERM=%s", NonNull(tname)));
> > 677 ret_error0(TGETENT_ERR, "TERM environment variable not set.\n");
> > 678 }
> > 679 #endif
> > 680 }
> > 681 myname = strdup(tname);
> > 682
> > 683 if (strlen(myname) > MAX_NAME_SIZE) {
> > 684 ret_error(TGETENT_ERR,
> > (gdb) p myname
> > $1 = 0x0
> > (gdb)
> > 
> > 
> > here is the solution.
> > solution:  add check for myname before use, patch: 
> > add-check-for-myname.patch
> > 
> > 
> > 
> > 
> > If there are any other ways to fix these problems,it will be better.
> > 
> > 
> > Looking forward to your reply, thanks.
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > | |
> > eaglegai
> > |
> > |
> > eaglegai@163.com
> > |
> 
> 
> 
> 
> 
> 
> 
> -- 
> Thomas E. Dickey <dickey@invisible-island.net>
> https://invisible-island.net



-- 
Thomas E. Dickey <dickey@invisible-island.net>
https://invisible-island.net

signature.asc
Description: PGP signature