Your message dated Wed, 8 Jul 2009 20:29:11 +1000
with message-id <[email protected]>
and subject line Re: Processed: libedit2: reassign 535519 358164 libbsd0
has caused the Debian Bug report #535519,
regarding libedit2: huge memory leak in fgetln emulation
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact [email protected]
immediately.)


-- 
535519: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=535519
Debian Bug Tracking System
Contact [email protected] with problems
--- Begin Message ---
Package: libedit2
Version: 2.9.cvs.20050518-2.2
Severity: important


Original semantic of fgetln(3) - return buffer owned by stdio (in fact,
it just passes pointer inside internal FILE* buffer), and user should
not free that buffer. Next call to stdio function may alter/free this
buffer. And upstream libedit follows that semantic.

Current emulation using getline(3) allocates new buffer each time, and this
buffer is never freed. This causes huge memory leak (for example, run
gnuplot [backport from squeeze or newer] under valgrind - it will report memory
leak on /.gnuplot_history and ~/.editrc files reading).

=== cut ===
==8915== 390,720 bytes in 3,256 blocks are definitely lost in loss record 45 of 
47
==8915==    at 0x401DAF8: malloc (vg_replace_malloc.c:207)
==8915==    by 0x41286CC: getdelim (in /lib/tls/i686/cmov/libc-2.3.6.so)
==8915==    by 0x4125E63: getline (in /lib/tls/i686/cmov/libc-2.3.6.so)
==8915==    by 0x4028436: fgetln (fgetln.c:8)
==8915==    by 0x40325FD: el_source (el.c:480)
==8915==    by 0x403655F: rl_initialize (readline.c:281)
==8915==    by 0x4037F54: using_history (readline.c:375)
==8915==    by 0x80923D3: (within /usr/bin/gnuplot)
==8915==    by 0x40E6EA7: (below main) (in /lib/tls/i686/cmov/libc-2.3.6.so)
==8915== 431,855 bytes in 1,730 blocks are definitely lost in loss record 47 of 
47
==8915==    at 0x401DBD2: realloc (vg_replace_malloc.c:429)
==8915==    by 0x4128643: getdelim (in /lib/tls/i686/cmov/libc-2.3.6.so)
==8915==    by 0x4125E63: getline (in /lib/tls/i686/cmov/libc-2.3.6.so)
==8915==    by 0x4028436: fgetln (fgetln.c:8)
==8915==    by 0x4034EDD: history (history.c:669)
==8915==    by 0x4036FB9: read_history (readline.c:1086)
==8915==    by 0x8092A1E: (within /usr/bin/gnuplot)
==8915==    by 0x40E6EA7: (below main) (in /lib/tls/i686/cmov/libc-2.3.6.so)
=== cut ===

Same bug present in all debian version of libedit - etch {2.9.cvs.20050518-2.2}
[verified], lenny {2.11~20080614-1} [presumed], squeeze/sid {2.11~20080614-2}
[verified on self-compiled backport].

This bug is linux-specific and not present in upstream (as upstream uses
native fgetln).

I've fixed that bug with attached patch (replaces original patch
debian/patches/06-fgetln.c-error.diff) [for etch one should replace header with
dpatch-compatible one].

My implementation is incomplete/incompatible too; as far as i can judge,
that should not matter for libedit uses, but better check yourself.

-- System Information:
Debian Release: 4.0
  APT prefers oldstable-proposed-updates
  APT policy: (500, 'oldstable-proposed-updates'), (500, 'oldstable')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.27.7-yk00
Locale: LANG=ru_RU.KOI8-R, LC_CTYPE=ru_RU.KOI8-R (charmap=KOI8-R)

Versions of packages libedit2 depends on:
ii  libc6                  2.3.6.ds1-13etch4 GNU C Library: Shared libraries
ii  libncurses5            5.5-5             Shared libraries for terminal hand

libedit2 recommends no packages.

-- no debconf information
 06-fgetln.c-memory-leak.diff by Yuriy Kaminskiy <[email protected]>

 Routine fgetln() leaked memory. Original semantic - stdio owns returned
 buffer (it is actually gives access directly to internal FILE buffer),
 and buffer should not be free()ed by user. New variant is still wrong -
 it is not thread-safe and uses single global buffer (instead of per-stream
 buffer), but at least it is  not leaking all memory anymore, and all libedit
 uses should be safe.

Index: libedit-2.11~20080614/glibc-bsd-glue/fgetln.c
===================================================================
--- libedit-2.11~20080614.orig/glibc-bsd-glue/fgetln.c	2009-07-02 07:43:44.000000000 +0400
+++ libedit-2.11~20080614/glibc-bsd-glue/fgetln.c	2009-07-02 07:56:50.000000000 +0400
@@ -3,11 +3,13 @@
 char *
 fgetln (FILE *stream, size_t *len)
 {
-	char *line=NULL;
+	static char *line;
+	static size_t linelen;
+	ssize_t res;
 
-	getline (&line, len, stream);
-
-	(*len)--; /* get rid of the trailing \0, fgetln
-	does not have it */
-	return line;
+	if ((res = getline (&line, &linelen, stream)) >= 0) {
+		*len = res;
+		return line;
+	} else
+		return NULL;
 }



--- End Message ---
--- Begin Message ---
Version: 2.11-20080614-1

On Wed, Jul 08, 2009 at 11:01:08AM +0200, Guillem Jover wrote:
>merge 358164 535519
>reassign 358164 libedit2
>close 358164 2.11-20080614-1
>thanks
>
>On Wed, 2009-07-08 at 07:03:04 +0000, Debian Bug Tracking System wrote:
>>Processing commands for [email protected]:
>>>reassign 535519 libbsd0
>>Bug#535519: libedit2: huge memory leak in fgetln emulation
>>Bug reassigned from package `libedit2' to `libbsd0'.
>>>reassign 358164 libbsd0
>>Bug#358164: libedit2: adding \n incrementally with read/write history
>>Bug reassigned from package `libedit2' to `libbsd0'.
>
>This bug got fixed when you switched to use libbsd, which has a working
>implementation of fgetln.
>
>Yuriy, when testing if a bug applies, please always do so against
>latest sid version, which in this case was 2.11-20080614-1, from
>2009-06-22. Thanks for the investigation and patch anyway!
>
>regards,
>guillem


--- End Message ---

Reply via email to