MAXPATHLEN in ncpfs

[Thomas Danckaert <post@thomasdanckaert.be>]

Hello,

attached patch fixes the MAXPATHLEN build problems for ncpfs.  Is this
the right mailing list for discussion, or should I send this message
to bug-hurd?  (Or directly submit a bug+patch to the Debian bug
tracking system?)

One issue with the patch, that I see, but don't know how to solve, is
the following: the char* mount_point is passed to library users as a
result of the function ncp_find_permanent(), exposed in the header
ncplib.h. Before, the function returned a pointer to the static
char[MAXPATHLEN] array, so the api user was not allowed to free() it,
but now it is a pointer to a malloc'ed buffer.  With this patch, the
buffer is free'd on every function call, so there should be no real
memory leak as far as i see.  However, it is not properly cleaned up
at program exit, and giving the user a pointer which is later free'd
might cause other problems as well (though the content of the array it
pointed to would have changed after every function call in the
original version as well, at least the pointer would have still been
valid).

kind regards,

Thomas Danckaert

Avoid dependence on MAXPATHLEN.

replace fixed-length character array ncp_conn_ent.mount_point by a
char*.


Index: ncpfs_fix/include/ncp/ncplib.h
===================================================================
--- ncpfs_fix.orig/include/ncp/ncplib.h	2013-09-07 15:12:30.000000000 +0200
+++ ncpfs_fix/include/ncp/ncplib.h	2013-09-07 16:59:04.454074383 +0200
@@ -393,7 +393,7 @@
 	fixedCharArray server[NCP_BINDERY_NAME_LEN];
 	char* user;
 	uid_t uid;
-	fixedCharArray mount_point[MAXPATHLEN];
+	char* mount_point;
 };
 #else
 struct ncp_conn_ent
@@ -401,7 +401,7 @@
 	char server[NCP_BINDERY_NAME_LEN];
 	char* user;
 	uid_t uid;
-	char mount_point[MAXPATHLEN];
+	char* mount_point;
 };
 #endif
 
Index: ncpfs_fix/lib/ncplib.c
===================================================================
--- ncpfs_fix.orig/lib/ncplib.c	2013-09-07 15:12:30.000000000 +0200
+++ ncpfs_fix/lib/ncplib.c	2013-09-07 17:04:32.982060643 +0200
@@ -2139,6 +2139,9 @@
 	struct mntent *mnt_ent;
 	int fid;
 
+        if( entry.mount_point != NULL ) {
+          free (entry.mount_point);
+        }
 	memset(server, 0, sizeof(server));
 	memset(&entry, 0, sizeof(entry));
 
@@ -2158,10 +2161,16 @@
 		*user = '\0';
 		user += 1;
 		entry.user = user;
-		if ((strlen(server) >= sizeof(entry.server))
-		    || (strlen(mnt_ent->mnt_dir) >= sizeof(entry.mount_point))) {
+		if (strlen(server) >= sizeof(entry.server)) {
 			continue;
 		}
+#ifdef MAXPATHLEN
+		if ( strlen(mnt_ent->mnt_dir) >= MAXPATHLEN) {
+			continue;
+		}
+#endif
+		entry.mount_point = malloc(strlen(mnt_ent->mnt_dir) + 1);
+
 		strcpy(entry.server, server);
 		strcpy(entry.mount_point, mnt_ent->mnt_dir);
 
@@ -2242,35 +2251,49 @@
 static NWCCODE
 ncp_fopen_nwc(FILE** nwc)
 {
-	char path[MAXPATHLEN];
+	char *path;
 	char *home = NULL;
 	struct stat st;
 	FILE* f;
+	int retval = 0;
+        int pathlen;
 
 	home = getenv("HOME");
-	if ((home == NULL)
-	    || (strlen(home) + sizeof(NWCLIENT) + 2 > sizeof(path))) {
+	if (home == NULL) {
 		return ENAMETOOLONG;
 	}
+        pathlen = strlen(home) + strlen(NWCLIENT) + 2;
+#ifdef MAXPATHLEN
+	if (pathlen > MAXPATHLEN) {
+		return ENAMETOOLONG;
+	}          
+#endif
+	path = malloc(pathlen);
 	strcpy(path, home);
 	strcat(path, "/");
 	strcat(path, NWCLIENT);
 
 	if (stat(path, &st) != 0) {
-		return errno;
+		retval = errno;
+		goto cleanup;
 	}
         if (st.st_uid != getuid()) {
-                return EACCES;
+                retval = EACCES;
+		goto cleanup;
         }
 	if ((st.st_mode & (S_IRWXO | S_IRWXG)) != 0) {
-		return NCPLIB_INVALID_MODE;
+		retval = NCPLIB_INVALID_MODE;
+		goto cleanup;
 	}
 	f = fopen(path, "r");
 	if (!f) {
-		return errno;
+		retval = errno;
+		goto cleanup;
 	}
 	*nwc = f;
-	return 0;
+ cleanup:
+	free(path);
+	return retval;
 }
 
 NWCCODE