Subject: Re: pkg/34798 (x11/mlterm: Dangerous file descriptor leaks)
To: None <uebayasi@NetBSD.org, gnats-admin@netbsd.org,>
From: Christian Biere <christianbiere@gmx.de>
List: pkgsrc-bugs
Date: 12/18/2006 16:35:02
The following reply was made to PR pkg/34798; it has been noted by GNATS.
From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/34798 (x11/mlterm: Dangerous file descriptor leaks)
Date: Mon, 18 Dec 2006 17:38:15 +0100
--9amGYk9869ThD9tj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
The attached file updates mlterm to 2.9.3 and provides updated patches for the
file descriptor leak issue and a potential off-by-one in an unrelated macro.
These patches have been extracted from mlterm's CVS repository.
The patch also removes the configure argument "--with-type-engines=xft" from
the Makefile. The default is "xcore" which works fine for me in contrast to
Xft.
--
Christian
--9amGYk9869ThD9tj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="mlterm.udif"
Index: Makefile
===================================================================
RCS file: /cvsroot/pkgsrc/x11/mlterm/Makefile,v
retrieving revision 1.46
diff -u -r1.46 Makefile
--- Makefile 12 Nov 2006 15:13:56 -0000 1.46
+++ Makefile 18 Dec 2006 16:19:02 -0000
@@ -1,7 +1,6 @@
# $NetBSD: Makefile,v 1.46 2006/11/12 15:13:56 hira Exp $
-DISTNAME= mlterm-2.9.2
-PKGREVISION= 4
+DISTNAME= mlterm-2.9.3
CATEGORIES= x11
MASTER_SITES= ${MASTER_SITE_SOURCEFORGE:=mlterm/}
@@ -27,7 +26,6 @@
.include "../../mk/bsd.prefs.mk"
CONFIGURE_ARGS+= --with-imagelib=gdk-pixbuf
-CONFIGURE_ARGS+= --with-type-engines=xft
CONFIGURE_ARGS+= --without-libiconv-prefix
CONFIGURE_ARGS+= --without-libintl-prefix
CONFIGURE_ARGS+= --sysconfdir=${PKG_SYSCONFBASE:Q}
Index: distinfo
===================================================================
RCS file: /cvsroot/pkgsrc/x11/mlterm/distinfo,v
retrieving revision 1.16
diff -u -r1.16 distinfo
--- distinfo 11 Jun 2005 15:00:04 -0000 1.16
+++ distinfo 18 Dec 2006 16:09:50 -0000
@@ -1,9 +1,18 @@
$NetBSD: distinfo,v 1.16 2005/06/11 15:00:04 hira Exp $
-SHA1 (mlterm-2.9.2.tar.gz) = d71f07bb38553a7cae927f15b8d0a20a165eebe7
-RMD160 (mlterm-2.9.2.tar.gz) = c83693c353f266333dc85588d8c1366925cca79c
-Size (mlterm-2.9.2.tar.gz) = 1980109 bytes
-SHA1 (patch-ad) = eaaded295f050591e6a30286163f832aae93fa3a
+SHA1 (mlterm-2.9.3.tar.gz) = 65ba5a7bd83accf7f621f84b84debb35c8f215ef
+RMD160 (mlterm-2.9.3.tar.gz) = 27f4381a8b8b68b07e53bbeb422581beb7527b48
+Size (mlterm-2.9.3.tar.gz) = 1984977 bytes
+SHA1 (patch-ad) = 4f6ef642002baa13bc4b7578cda0e45888fed27f
SHA1 (patch-ae) = fde46b06d0dfb2c296c91a69e9e1f8ff11d68766
SHA1 (patch-af) = b9947c2a817093b71e3e91312bac173d5106c306
SHA1 (patch-ag) = 7f2ff30719dd9baecda31627b45f9516225ec602
+SHA1 (patch-ah) = 93b0ad1445faec91d29f3a01b69d9ac0f4ddfe90
+SHA1 (patch-ai) = 7de76af14e77dc540e2d0692682102fd078686ff
+SHA1 (patch-aj) = ff103698ff544fa22fa660268dccb51b8b9e8bda
+SHA1 (patch-ak) = 3b06cfc41ed12be11ea81d097f0592bb6b977e29
+SHA1 (patch-al) = 6b87b81e403a3fac9794c666c1e0260a3d5b5bd4
+SHA1 (patch-am) = 83684bfa15e5596abddef92cf994bb92b9f21990
+SHA1 (patch-an) = 156b8ebf5c7fdf68c39df7076849a05a90eb7e5a
+SHA1 (patch-ao) = 680ac866197202e09c25d93b20a7e3b700f38370
Index: patches/patch-ad
===================================================================
RCS file: /cvsroot/pkgsrc/x11/mlterm/patches/patch-ad,v
retrieving revision 1.1
diff -u -r1.1 patch-ad
--- patches/patch-ad 6 Mar 2005 17:50:49 -0000 1.1
+++ patches/patch-ad 18 Dec 2006 16:09:50 -0000
@@ -1,7 +1,7 @@
-$NetBSD: patch-ad,v 1.1 2005/03/06 17:50:49 hira Exp $
+$NetBSD$
---- kiklib/src/kik_pty_streams.c.orig 2004-10-23 06:59:39.000000000 +0900
-+++ kiklib/src/kik_pty_streams.c 2005-03-05 23:07:02.000000000 +0900
+--- kiklib/src/kik_pty_streams.c.orig 2005-11-21 15:24:03.000000000 +0100
++++ kiklib/src/kik_pty_streams.c 2006-12-16 00:04:11.000000000 +0100
@@ -9,6 +9,7 @@
/* When _XOPEN_SOURCE is defined,
* u_int/u_long/... may not be defined without _BSD_SOURCE */
@@ -20,7 +20,15 @@
#include "kik_str.h" /* strdup */
#include "kik_debug.h"
-@@ -99,13 +103,30 @@
+@@ -75,6 +79,7 @@
+ kik_msg_printf( "Unable to open a master pseudo-terminal device.\n") ;
+ return -1;
+ }
++ kik_file_set_cloexec( *master) ;
+ /*
+ * The behaviour of the grantpt() function is unspecified
+ * if the application has installed a signal handler to catch SIGCHLD signals.
+@@ -104,13 +109,30 @@
return -1;
}
@@ -53,7 +61,7 @@
/*
* cygwin doesn't have isastream.
*/
-@@ -204,78 +225,13 @@
+@@ -209,79 +231,16 @@
}
}
@@ -136,5 +144,8 @@
+ kik_warn_printf( KIK_DEBUG_TAG " tcsetattr() failed.\n") ;
+ #endif
}
++
++ kik_file_set_cloexec( *slave) ;
return pid ;
+ }
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-ah 2006-12-16 00:41:32.000000000 +0100
@@ -0,0 +1,39 @@
+$NetBSD$
+
+safer DIGIT_STR_LEN().
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/kiklib/src/kik_util.h?r1=1.4&r2=1.5&view=patch
+
+--- kiklib/src/kik_util.h.orig 2002/02/02 09:20:27 1.4
++++ kiklib/src/kik_util.h 2006/10/13 16:13:42 1.5
+@@ -10,19 +10,20 @@
+
+ #define K_MIN(n1,n2) ((n1) > (n2) ? (n2) : (n1))
+
+-/*
+- * char : 0 - 256 (3)
+- * int16 : 0 - 65536 (5)
+- * int32 : 0 - 4294967296 (10)
+- * int64 : 0 - 18446744073709551616 (20)
++/* TYPE: MIN(signed) -- MAX(unsigned) (number of bytes needed)
++ * char : -128 -- 256 (4)
++ * int16 : -32768 -- 65536 (6)
++ * int32 : -2147483648 -- 4294967296 (11)
++ * int64 : -9223372036854775808 -- 18446744073709551616 (20)
+ *
+- * 40 is evenly selected in other cases just to be sure.
++ * Since log10(2^8) = 2.4..., (sizeof(n)*3) is large enough
++ * for all n >= 2.
+ */
+ #define DIGIT_STR_LEN(n) \
+- ((sizeof(n) == 1) ? 3 : \
+- (sizeof(n) == 2) ? 5 : \
+- (sizeof(n) == 4) ? 10 : \
+- (sizeof(n) == 8) ? 20 : 40)
++ ((sizeof(n) == 1) ? 4 : \
++ (sizeof(n) == 2) ? 6 : \
++ (sizeof(n) == 4) ? 11 : \
++ (sizeof(n) == 8) ? 20 : (sizeof(n)*3))
+
+
+ #endif
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-ai 2006-12-16 01:25:22.000000000 +0100
@@ -0,0 +1,60 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/kiklib/src/kik_file.c?r1=1.5&r2=1.6&view=patch
+
+--- kiklib/src/kik_file.c.orig 2003-01-12 13:34:31.000000000 +0100
++++ kiklib/src/kik_file.c 2006-12-16 01:22:10.000000000 +0100
+@@ -2,6 +2,7 @@
+ * $Id: kik_file.c,v 1.5 2003/01/12 12:34:31 arakiken Exp $
+ */
+
++#include <fcntl.h> /* fcntl() */
+ #include "kik_file.h"
+
+ #include "kik_config.h"
+@@ -200,3 +201,43 @@
+ }
+
+ #endif
++
++int
++kik_file_set_cloexec(
++ int fd
++ )
++{
++ int old_flags ;
++
++ old_flags = fcntl( fd, F_GETFD) ;
++ if( old_flags == -1)
++ {
++ return 0 ;
++ }
++
++ if( !(old_flags & FD_CLOEXEC)
++ && (fcntl( fd, F_SETFD, old_flags|FD_CLOEXEC) == -1) )
++ {
++ return 0 ;
++ }
++ return 1 ;
++}
++
++kik_file_unset_cloexec(
++ int fd
++ )
++{
++ int old_flags ;
++
++ old_flags = fcntl( fd, F_GETFD) ;
++ if( old_flags == -1)
++ {
++ return 0 ;
++ }
++ if( (old_flags & FD_CLOEXEC)
++ && (fcntl( fd, F_SETFD, old_flags & (~FD_CLOEXEC)) == -1) )
++ {
++ return 0 ;
++ }
++ return 1 ;
++}
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-aj 2006-12-16 00:41:39.000000000 +0100
@@ -0,0 +1,17 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/kiklib/src/kik_file.h?r1=1.5&r2=1.6&view=patch
+
+--- kiklib/src/kik_file.h.orig 2003/01/12 12:34:31 1.5
++++ kiklib/src/kik_file.h 2006/10/19 14:41:51 1.6
+@@ -33,5 +33,8 @@
+
+ int kik_file_unlock( int fd) ;
+
++int kik_file_set_cloexec( int fd) ;
++
++int kik_file_unset_cloexec( int fd) ;
+
+ #endif
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-ak 2006-12-16 00:41:47.000000000 +0100
@@ -0,0 +1,40 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/kiklib/src/kik_pty_bsd.c?r1=1.9&r2=1.10&view=patch
+
+--- kiklib/src/kik_pty_bsd.c.orig 2003/03/08 03:19:29 1.9
++++ kiklib/src/kik_pty_bsd.c 2006/10/19 14:41:51 1.10
+@@ -118,6 +118,7 @@
+ }
+ else
+ {
++ kik_file_set_cloexec( *master) ;
+ /*
+ * we succeeded to open pty master.
+ * opening pty slave in succession.
+@@ -136,6 +137,7 @@
+ }
+ else
+ {
++ kik_file_set_cloexec( *slave) ;
+ return 1 ;
+ }
+ }
+@@ -195,7 +197,6 @@
+ /*
+ * parent process
+ */
+-
+ /*
+ * delaying.
+ */
+@@ -301,5 +302,7 @@
+ #endif
+ }
+
++ kik_file_set_cloexec( *slave) ;
++
+ return pid ;
+ }
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-al 2006-12-16 00:41:52.000000000 +0100
@@ -0,0 +1,52 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/xwindow/x_xim.c?r1=1.9&r2=1.11&view=patch
+
+--- xwindow/x_xim.c.orig 2005/02/10 15:15:08 1.9
++++ xwindow/x_xim.c 2006/10/23 13:09:36 1.11
+@@ -5,6 +5,8 @@
+
+ #include "x_xim.h"
+
+ #include <stdio.h> /* sprintf */
+ #include <string.h> /* strcmp/memset */
++#include <unistd.h> /* dup/close */
++#include <kiklib/kik_file.h> /* kik_set_file_cloexec */
+ #include <kiklib/kik_debug.h>
+@@ -121,6 +123,7 @@
+ char * xmod ;
+ char * cur_locale ;
+ int result ;
++ int next_fd ; /* to deal with brain-dead XIM implemantations */
+
+ /* 4 is the length of "@im=" */
+ if( ( xmod = alloca( 4 + strlen( xim->name) + 1)) == NULL)
+@@ -154,6 +157,12 @@
+
+ result = 0 ;
+
++ next_fd = dup( 0) ;
++ if( next_fd != -1)
++ {
++ /* remember the lowest unused fd */
++ close( next_fd) ;
++ }
+ if( XSetLocaleModifiers(xmod) && ( xim->im = XOpenIM( display , NULL , NULL , NULL)))
+ {
+ if( ( xim->encoding = ml_get_char_encoding( kik_get_codeset())) == ML_UNKNOWN_ENCODING ||
+@@ -172,7 +181,12 @@
+ result = 1 ;
+ }
+ }
+-
++ if( next_fd > 0)
++ {
++ /* if XOpenIM() internally opens a fd,
++ * we should close it on exec() */
++ kik_file_set_cloexec( next_fd) ;
++ }
+ if( cur_locale)
+ {
+ /* restoring */
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-am 2006-12-16 00:41:59.000000000 +0100
@@ -0,0 +1,56 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/mlterm/ml_config_menu.c?r1=1.6&r2=1.7&view=patch
+
+--- mlterm/ml_config_menu.c.orig 2003/09/16 03:18:31 1.6
++++ mlterm/ml_config_menu.c 2006/10/19 14:41:51 1.7
+@@ -33,6 +33,7 @@
+ if( config_menu->pid == pid)
+ {
+ config_menu->pid = 0 ;
++ close( config_menu->fd) ;
+ config_menu->fd = -1 ;
+ }
+ }
+@@ -83,6 +84,12 @@
+ return 0 ;
+ }
+
++ if( !kik_file_unset_cloexec( pty_fd))
++ {
++ /* configulators require an inherited pty. */
++ return 0 ;
++ }
++
+ if( pipe( fds) == -1)
+ {
+ return 0 ;
+@@ -131,8 +138,14 @@
+
+ close( fds[1]) ;
+
+- if( dup2( fds[0] , STDIN_FILENO) == -1 || dup2( pty_fd , STDOUT_FILENO) == -1 ||
+- execv( cmd_path , args) == -1)
++ /* for configulators,
++ * STDIN => to read replys from mlterm
++ * STDOUT => to write the "master" side of pty
++ * STDERR => is retained to be the mlterm's STDERR
++ */
++ if( dup2( fds[0] , STDIN_FILENO) == -1
++ || dup2( pty_fd , STDOUT_FILENO) == -1
++ || execv( cmd_path , args) == -1)
+ {
+ /* failed */
+
+@@ -148,6 +161,9 @@
+
+ config_menu->fd = fds[1] ;
+ config_menu->pid = pid ;
++
++ kik_file_set_cloexec( pty_fd) ;
++ kik_file_set_cloexec( config_menu->fd) ;
+
+ return 1 ;
+ }
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-an 2006-12-16 00:42:10.000000000 +0100
@@ -0,0 +1,26 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/xwindow/x_display.c?r1=1.4&r2=1.5&view=patch
+
+--- xwindow/x_display.c.orig 2003/12/09 11:56:13 1.4
++++ xwindow/x_display.c 2006/10/19 14:41:51 1.5
+@@ -7,6 +7,7 @@
+ #include <kiklib/kik_debug.h>
+ #include <kiklib/kik_mem.h>
+ #include <kiklib/kik_str.h> /* strdup */
++#include <kiklib/kik_file.h> /* kik_file_set_cloexec */
+
+ #include "x_xim.h"
+ #include "x_picture.h"
+@@ -39,6 +40,9 @@
+ goto error1 ;
+ }
+
++ /* set close-on-exec flag on the socket connected to X. */
++ kik_file_set_cloexec( XConnectionNumber( disp->display));
++
+ if( ( disp->name = strdup( name)) == NULL)
+ {
+ goto error2 ;
--- /dev/null 2006-12-18 17:14:33.000000000 +0100
+++ patches/patch-ao 2006-12-16 00:42:17.000000000 +0100
@@ -0,0 +1,35 @@
+$NetBSD$
+
+plugged fd leaks.
+
+http://mlterm.cvs.sourceforge.net/mlterm/mlterm/xwindow/x_term_manager.c?r1=1.93&r2=1.95&view=patch
+
+--- xwindow/x_term_manager.c.orig 2006/09/30 16:09:44 1.93
++++ xwindow/x_term_manager.c 2006/11/20 11:22:34 1.95
+@@ -872,7 +872,8 @@
+ {
+ return -1 ;
+ }
+-
++ kik_file_set_cloexec( sock_fd);
++
+ while( bind( sock_fd , (struct sockaddr *) &servaddr , sizeof( servaddr)) < 0)
+ {
+ if( errno == EADDRINUSE)
+@@ -993,7 +994,7 @@
+ * If this flag off, this fd remained open until the child process forked in
+ * open_screen_intern()(ml_term_open_pty()) close it.
+ */
+- fcntl( fd , F_SETFD , 1) ;
++ kik_file_set_cloexec( fd) ;
+
+ if( ( fp = fdopen( fd , "r+")) == NULL)
+ {
+@@ -1855,6 +1856,7 @@
+ {
+ additional_fds[i].fd = fd ;
+ additional_fds[i].handler = handler ;
++ kik_file_set_cloexec( fd);
+
+ return 0 ;
+ }
--9amGYk9869ThD9tj--