NetBSD Problem Report #54654

From www@netbsd.org  Tue Oct 29 10:10:25 2019
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 709417A159
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 29 Oct 2019 10:10:25 +0000 (UTC)
Message-Id: <20191029101024.64F077A279@mollari.NetBSD.org>
Date: Tue, 29 Oct 2019 10:10:24 +0000 (UTC)
From: soeren@soeren-tempel.net
Reply-To: soeren@soeren-tempel.net
To: gnats-bugs@NetBSD.org
Subject: libedit: segfault in re_fastputc
X-Send-Pr-Version: www-1.0

>Number:         54654
>Category:       lib
>Synopsis:       libedit: segfault in re_fastputc
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    christos
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 29 10:15:01 +0000 2019
>Last-Modified:  Sat Apr 17 07:46:07 +0000 2021
>Originator:     S÷ren Tempel
>Release:        libedit from 2019-10-25
>Organization:
>Environment:
Unfortunately, I use the portable version of netbsd libedit that Jess Thrys°e distributes on Linux. However, I verified that the problem wasn't fix in NetBSD CVS yet.
>Description:
When using libedit-enabled programs in a one-line terminal window (e.g. created using `tmux split-window -l 1`) horizontal scrolling does not work correctly if the text exceeds this one line. For example, scrolling left after exceeding this line will cause no text to be displayed. A combination of the Left Arrow, C-a, C-e and C-r key binding will then cause a segmentation fault.

The code causing the segfault seems to be refresh.c:1101:

    el->el_display[el->el_cursor.v][el->el_cursor.h++] = c;

Presumably the horizontal cursor position causes the issue.

A valgrind output for this issue and libedit compiled with `-DDEBUG_REFRESH` can be found here: https://gist.github.com/nmeum/d76e42cf9cae84cba9bfd5d71bd83a8a

If you don't have webbrowsing capabilities, the valgrind output without `-DDEBUG_REFRESH` is way shorter and looks as follows:

    ==29962== Memcheck, a memory error detector
    ==29962== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
    ==29962== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
    ==29962== Command: input 
    ==29962== 
    ==29962== Invalid write of size 4
    ==29962==    at 0x4CC2C25: re_fastputc (refresh.c:1101)
    ==29962==    by 0x4CC2F2F: re_fastaddc (refresh.c:1170)
    ==29962==    by 0x4CB65D3: ed_insert (common.c:96)
    ==29962==    by 0x4CC0BA3: el_wgets (read.c:537)
    ==29962==    by 0x4CB979A: el_gets (eln.c:75)
    ==29962==    by 0x10AA61: iloop (input.c:156)
    ==29962==    by 0x10AFC0: main (input.c:286)
    ==29962==  Address 0x38 is not stack'd, malloc'd or (recently) free'd
    ==29962== 
    ==29962== 
    ==29962== Process terminating with default action of signal 11 (SIGSEGV)
    ==29962==  Access not within mapped region at address 0x38
    ==29962==    at 0x4CC2C25: re_fastputc (refresh.c:1101)
    ==29962==    by 0x4CC2F2F: re_fastaddc (refresh.c:1170)
    ==29962==    by 0x4CB65D3: ed_insert (common.c:96)
    ==29962==    by 0x4CC0BA3: el_wgets (read.c:537)
    ==29962==    by 0x4CB979A: el_gets (eln.c:75)
    ==29962==    by 0x10AA61: iloop (input.c:156)
    ==29962==    by 0x10AFC0: main (input.c:286)
    ==29962==  If you believe this happened as a result of a stack
    ==29962==  overflow in your program's main thread (unlikely but
    ==29962==  possible), you can try to increase the size of the
    ==29962==  main thread stack using the --main-stacksize= flag.
    ==29962==  The main thread stack size used in this run was 8388608.
    ==29962== 
    ==29962== HEAP SUMMARY:
    ==29962==     in use at exit: 71,355 bytes in 127 blocks
    ==29962==   total heap usage: 196 allocs, 69 frees, 87,509 bytes allocated
    ==29962== 
    ==29962== LEAK SUMMARY:
    ==29962==    definitely lost: 0 bytes in 0 blocks
    ==29962==    indirectly lost: 0 bytes in 0 blocks
    ==29962==      possibly lost: 0 bytes in 0 blocks
    ==29962==    still reachable: 70,839 bytes in 121 blocks
    ==29962==         suppressed: 516 bytes in 6 blocks
    ==29962== Rerun with --leak-check=full to see details of leaked memory
    ==29962== 
    ==29962== For lists of detected and suppressed errors, rerun with: -s
    ==29962== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Output is from a custom libedit-enabled program, but I was able to reproduce this with other programs, e.g. sftp as well.
>How-To-Repeat:
1. Start tmux
2. Create a one-line tmux pane using tmux split-window -l 1 (requires at least tmux 2.9 with the following patch https://cvsweb.openbsd.org/src/usr.bin/tmux/tty.c?rev=1.307&content-type=text/x-cvsweb-markup)
3. Start a libedit-enabled program in this newly created tmux split (e.g. sftp)
4. Fill the line with bogus characters until horizontal scrolling is forced
5. Scroll left horizontally to see the previous input (should be empty)
6. Try a combination of the Left Arrow, C-a, C-e and C-r key binding until it crashes, maybe types some additional characters in between
>Fix:
None.

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54654 CVS commit: src/lib/libedit
Date: Tue, 12 Nov 2019 15:59:46 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Tue Nov 12 20:59:46 UTC 2019

 Modified Files:
 	src/lib/libedit: terminal.c

 Log Message:
 PR/54654: Soren Tempel: Make sure el_cursor.v < el_terminal.t_size.v when
 moving around.


 To generate a diff of this commit:
 cvs rdiff -u -r1.40 -r1.41 src/lib/libedit/terminal.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: soeren@soeren-tempel.net
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/54654 CVS commit: src/lib/libedit
Date: Wed, 20 Nov 2019 10:14:35 +0100

 "Christos Zoulas" <christos@netbsd.org> wrote:
 >  Module Name:	src
 >  Committed By:	christos
 >  Date:		Tue Nov 12 20:59:46 UTC 2019
 > =20
 >  Modified Files:
 >  	src/lib/libedit: terminal.c
 > =20
 >  Log Message:
 >  PR/54654: Soren Tempel: Make sure el_cursor.v < el_terminal.t_size.v whe=
 n
 >  moving around.

 Thanks for the quick fix, I can confirm that this change fixes the
 segmentation fault, though horizontal scrolling in one-line terminal
 still doesn't seem to work correctly. That is, previous characters are
 not shown when scrolling left after filling the entire line.

 Cheers,
 S=C3=B6ren

Responsible-Changed-From-To: lib-bug-people->christos
Responsible-Changed-By: maya@NetBSD.org
Responsible-Changed-When: Sat, 17 Apr 2021 07:46:07 +0000
Responsible-Changed-Why:
Hi christos, the reporter mentions that there are still issues. Is this expected? (should we create another bug for this problem)?


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.