NetBSD Problem Report #47858

From www@NetBSD.org  Sun May 26 21:08:58 2013
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AB5A770D04
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 26 May 2013 21:08:58 +0000 (UTC)
Message-Id: <20130526210857.06EFA70D0A@mollari.NetBSD.org>
Date: Sun, 26 May 2013 21:08:57 +0000 (UTC)
From: linasvepstas@gmail.com
Reply-To: linasvepstas@gmail.com
To: gnats-bugs@NetBSD.org
Subject: libedit widechar version mishandles EOF
X-Send-Pr-Version: www-1.0

>Number:         47858
>Category:       lib
>Synopsis:       libedit widechar version mishandles EOF
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 26 21:10:00 +0000 2013
>Last-Modified:  Wed May 29 18:30:01 +0000 2013
>Originator:     Linas Vepstas
>Release:        libedit
>Organization:
>Environment:
>Description:
libedit, when built with widechar support (UTF-8/unicode), will spin in infinite loop in el_wgets() when getting an EOF. 

To reproduce, cat a file to a program using el_wgets().  At the end, the program will spin up to 100% CPU, stuck in a read-loop.

Bug is easy to find: in src/lib/libedit/src/read.c lines 323 to 347 there is a failure to check for EOF (num_read==0) and as a result, the thing spins.  Patch below.

--- libedit/src/read.c.orig   2013-05-26 15:55:23.000000000 -0500
+++ libedit/src/read.c  2013-05-26 16:04:52.000000000 -0500
@@ -341,6 +341,13 @@
      }
   }

+  /* Test for EOF */
+  if (num_read == 0) {
+     errno = 0;
+     *cp = '\0';
+     return 0;
+  }
+
 #ifdef WIDECHAR
   if (el->el_flags & CHARSET_IS_UTF8) {
      if (!utf8_islead((unsigned char)cbuf[0]))

>How-To-Repeat:

>Fix:
See patch provided above.

>Audit-Trail:
From: Linas Vepstas <linasvepstas@gmail.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/47858: libedit widechar version mishandles EOF
Date: Sun, 26 May 2013 16:25:19 -0500

 Just to be clear, here is a closer description of the problem.  The
 offending lines are these:

 #ifdef WIDECHAR
    if (el->el_flags & CHARSET_IS_UTF8) {
       if (!utf8_islead((unsigned char)cbuf[0]))
          goto again; /* discard the byte we read and try again */

 When an EOF is read, cbuf[0] contains some sort of undefined garbage,
 left over from a previous read, thus the goto again is taken.  The
 non-widechar version doesn't have this problem, and thus perhaps my
 earlier patch is too general/too broad.  I believe it's a great patch,
 but if you don't like it, the below is a more conservative patch:

 --- libedit/src/read.c.orig   2013-05-26 15:55:23.000000000 -0500
 +++ libedit/src/read.c  2013-05-26 16:23:45.000000000 -0500
 @@ -342,6 +342,13 @@
    }

  #ifdef WIDECHAR
 +  /* Test for EOF; cbuf[0] holds garbage when num_read == 0 */
 +  if (num_read == 0) {
 +     errno = 0;
 +     *cp = '\0';
 +     return 0;
 +  }
 +
    if (el->el_flags & CHARSET_IS_UTF8) {
       if (!utf8_islead((unsigned char)cbuf[0]))
          goto again; /* discard the byte we read and try again */

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, lib-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, linasvepstas@gmail.com
Cc: 
Subject: Re: lib/47858: libedit widechar version mishandles EOF
Date: Wed, 29 May 2013 14:29:51 -0400

 On May 26,  9:30pm, linasvepstas@gmail.com (Linas Vepstas) wrote:
 -- Subject: Re: lib/47858: libedit widechar version mishandles EOF

 |  Just to be clear, here is a closer description of the problem.  The
 |  offending lines are these:
 |  
 |  #ifdef WIDECHAR
 |     if (el->el_flags & CHARSET_IS_UTF8) {
 |        if (!utf8_islead((unsigned char)cbuf[0]))
 |           goto again; /* discard the byte we read and try again */
 |  
 |  When an EOF is read, cbuf[0] contains some sort of undefined garbage,
 |  left over from a previous read, thus the goto again is taken.  The
 |  non-widechar version doesn't have this problem, and thus perhaps my
 |  earlier patch is too general/too broad.  I believe it's a great patch,
 |  but if you don't like it, the below is a more conservative patch:

 I understand, I moved it outside the WIDECHAR conditional because it
 should apply to the normal case too.

 thanks,

 christos

Home	PR Database Search