[krbdev.mit.edu #5597] ftp: error reading remote file on passive get results in leaked descriptor

nalin@redhat.com via RT rt-comment at krbdev.mit.edu
Mon Sep 17 18:17:38 EDT 2007


Hmm, my previously-proposed patch turned out to miss the case where the
descriptor is open and is the temporarly list of files to be retrieved
during an "mget".  This revised version sets the variable which holds
that descriptor to INVALID_SOCKET when that happens.  This requires
storing a copy of the descriptor elsewhere in the block where we pass it
to FDOPEN_SOCKET twice.

On non-Win32 boxes, it's also passed to dup() first, so that the two
resulting FILE structures don't share the same underlying descriptor.
I'm not sure if that's _really_ necessary, but it felt more correct.

Index: src/appl/gssftp/ftp/ftp_var.h
===================================================================
--- src/appl/gssftp/ftp/ftp_var.h	(revision 19936)
+++ src/appl/gssftp/ftp/ftp_var.h	(working copy)
@@ -48,7 +48,8 @@
 #define PERROR_SOCKET(str) do { errno = SOCKET_ERRNO; perror(str); } while(0)
 #else
 #define FCLOSE_SOCKET(f) fclose(f)
-#define FDOPEN_SOCKET(s, mode) fdopen(s, mode)
+FILE* fdopen_socket(int *s, char* mode);
+#define FDOPEN_SOCKET(s, mode) fdopen_socket(&s, mode)
 #define SOCKETNO(fd) (fd)
 #define PERROR_SOCKET(str) perror(str)
 #endif
Index: src/appl/gssftp/ftp/ftp.c
===================================================================
--- src/appl/gssftp/ftp/ftp.c	(revision 19936)
+++ src/appl/gssftp/ftp/ftp.c	(working copy)
@@ -196,7 +196,7 @@
 hookup(char* host, int port)
 {
 	register struct hostent *hp = 0;
-	int s;
+	int s, t;
 	socklen_t len;
 #ifdef IP_TOS
 #ifdef IPTOS_LOWDELAY
@@ -274,8 +274,13 @@
 	}
 #endif
 #endif
+#ifndef _WIN32
+	t = dup(s);
+#else
+	t = s;
+#endif
 	cin = FDOPEN_SOCKET(s, "r");
-	cout = FDOPEN_SOCKET(s, "w");
+	cout = FDOPEN_SOCKET(t, "w");
 	if (cin == NULL || cout == NULL) {
 		fprintf(stderr, "ftp: fdopen failed.\n");
 		if (cin) {
@@ -1450,6 +1455,8 @@
 	int a1,a2,a3,a4,p1,p2;
 
 	if (passivemode) {
+		if (data != INVALID_SOCKET)
+			(void) closesocket(data);
 		data = socket(AF_INET, SOCK_STREAM, 0);
 		if (data == INVALID_SOCKET) {
 			PERROR_SOCKET("ftp: socket");
@@ -2365,4 +2372,16 @@
 
 	return f;
 }
+#else
+/* Non-Win32 case takes the address of the variable so that we can "take
+ * ownership" of the descriptor number. */
+FILE* fdopen_socket(int *s, char* mode)
+{
+	FILE *fp;
+	fp = fdopen(*s, mode);
+	if (fp) {
+		*s = INVALID_SOCKET;
+	}
+	return fp;
+}
 #endif /* _WIN32 */




More information about the krb5-bugs mailing list