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 <yum...@gmail.com>

 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;
 }


Reply via email to