[patch] smbmount fixes for samba-2.0.7, testers wanted!

Urban Widmark urban at svenskatest.se
Mon Jul 31 16:30:51 GMT 2000


Hello all

This patch tries to remove a few bugs from smbmount. It would be nice to
get some testing+feedback on this from others using smbmount and possibly
get these things fixed for the next release of samba.


It does the following:

* Change lib/debug.c to allow changing your mind on being interactive.
  A second call to setup_logging should now replace the effects of a
  previous call. (hmm, it probably only works for interactive -> non
  interactive)

* Makes DEBUG() work in smbmount, even if running from autofs. Redirect
  stdout & stderr to /dev/null (well, it's better than to die because we
  wrote to a closed file).

  Once smbmount has done the initial mount, any debug output will be put
  where smb.conf says that log output should go. Maybe it should send all
  log output there but this way minimizes the effect a user sees.

* Replace fprintf(stderr, ... with DEBUG(0, ... to make them work after
  smbmount has become a daemon.

* Close the fd on the mountpoint if the ioctl fails, else the umount will
  also fail because the mountpoint is busy.

* Close the socket after the ioctl. Removes a socket and memory leak.
  (Roderich Schupp, Debian bug #53149
   http://cgi.debian.org/cgi-bin/bugreport.cgi?archive=no&bug=53149)

* Handle do_connection failures in send_fs_socket, else smbmount will
  coredump on a null 'c' pointer.

* Abort the mount if smbmnt fails.

* Swap order of lp_load() and parse_mount_smb() to allow commandline to
  override some flags (like debug level). This is similar to
  client/client.c


/Urban
-------------- next part --------------
--- samba-2.0.7-orig/source/lib/debug.c	Wed Apr 26 01:06:50 2000
+++ samba-2.0.7/source/lib/debug.c	Mon Jul 31 15:43:25 2000
@@ -168,6 +168,10 @@
  */
 void setup_logging( char *pname, BOOL interactive )
   {
+  /* reset to allow multiple setup calls */
+  stdout_logging = False;
+  dbf = NULL;
+
   if( interactive )
     {
     stdout_logging = True;
--- samba-2.0.7-orig/source/client/smbmount.c	Wed Apr 26 01:06:42 2000
+++ samba-2.0.7/source/client/smbmount.c	Mon Jul 31 16:25:00 2000
@@ -27,18 +27,12 @@
 #include <asm/types.h>
 #include <linux/smb_fs.h>
 
-/* Uncomment this to allow debug the mount.smb daemon */
-/* WARNING!  This option is incompatible with autofs/automount because
-	it does not close the stdout pipe back to the automount
-	process, which automount depends on.  This will cause automount
-	to hang!  Use with caution! */
-/* #define SMBFS_DEBUG 1 */
-
 extern struct in_addr ipzero;
 extern int DEBUGLEVEL;
 
 extern BOOL in_client;
 extern pstring user_socket_options;
+extern fstring remote_machine;
 
 static pstring my_netbios_name;
 static pstring password;
@@ -73,7 +67,7 @@
 	signal( SIGTERM, exit_parent );
 
 	if ((child_pid = fork()) < 0) {
-		fprintf(stderr,"could not fork\n");
+		DEBUG(0,("could not fork\n"));
 	}
 
 	if (child_pid > 0) {
@@ -151,12 +145,12 @@
 	/* have to open a new connection */
 	if (!(c=cli_initialise(NULL)) || (cli_set_port(c, smb_port) == 0) ||
 	    !cli_connect(c, server_n, &ip)) {
-		fprintf(stderr,"Connection to %s failed\n", server_n);
+		DEBUG(0,("Connection to %s failed\n", server_n));
 		return NULL;
 	}
 
 	if (!cli_session_request(c, &calling, &called)) {
-		fprintf(stderr, "session request to %s failed\n", called.name);
+		DEBUG(0,("session request to %s failed\n", called.name));
 		cli_shutdown(c);
 		if (strcmp(called.name, "*SMBSERVER")) {
 			make_nmb_name(&called , "*SMBSERVER", 0x20);
@@ -168,7 +162,7 @@
 	DEBUG(4,(" session request ok\n"));
 
 	if (!cli_negprot(c)) {
-		fprintf(stderr, "protocol negotiation failed\n");
+		DEBUG(0,("protocol negotiation failed\n"));
 		cli_shutdown(c);
 		return NULL;
 	}
@@ -184,7 +178,7 @@
 			       password, strlen(password),
 			       password, strlen(password),
 			       workgroup)) {
-		fprintf(stderr, "session setup failed: %s\n", cli_errstr(c));
+		DEBUG(0,("session setup failed: %s\n", cli_errstr(c)));
 		return NULL;
 	}
 
@@ -192,7 +186,7 @@
 
 	if (!cli_send_tconX(c, share, "?????",
 			    password, strlen(password)+1)) {
-		fprintf(stderr,"tree connect failed: %s\n", cli_errstr(c));
+		DEBUG(0,("tree connect failed: %s\n", cli_errstr(c)));
 		cli_shutdown(c);
 		return NULL;
 	}
@@ -226,29 +220,29 @@
 		the lights to exit anyways...
 	*/
         if (umount(mount_point) != 0) {
-                fprintf(stderr, "Could not umount %s: %s\n",
-                        mount_point, strerror(errno));
+                DEBUG(0,("Could not umount %s: %s\n",
+			 mount_point, strerror(errno)));
                 return;
         }
 
         if ((fd = open(MOUNTED"~", O_RDWR|O_CREAT|O_EXCL, 0600)) == -1) {
-                fprintf(stderr, "Can't get "MOUNTED"~ lock file");
+                DEBUG(0,("Can't get "MOUNTED"~ lock file"));
                 return;
         }
 
         close(fd);
 	
         if ((mtab = setmntent(MOUNTED, "r")) == NULL) {
-                fprintf(stderr, "Can't open " MOUNTED ": %s\n",
-                        strerror(errno));
+                DEBUG(0,("Can't open " MOUNTED ": %s\n",
+			 strerror(errno)));
                 return;
         }
 
 #define MOUNTED_TMP MOUNTED".tmp"
 
         if ((new_mtab = setmntent(MOUNTED_TMP, "w")) == NULL) {
-                fprintf(stderr, "Can't open " MOUNTED_TMP ": %s\n",
-                        strerror(errno));
+                DEBUG(0,("Can't open " MOUNTED_TMP ": %s\n",
+			 strerror(errno)));
                 endmntent(mtab);
                 return;
         }
@@ -262,21 +256,21 @@
         endmntent(mtab);
 
         if (fchmod (fileno (new_mtab), S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) < 0) {
-                fprintf(stderr, "Error changing mode of %s: %s\n",
-                        MOUNTED_TMP, strerror(errno));
+                DEBUG(0,("Error changing mode of %s: %s\n",
+			 MOUNTED_TMP, strerror(errno)));
                 return;
         }
 
         endmntent(new_mtab);
 
         if (rename(MOUNTED_TMP, MOUNTED) < 0) {
-                fprintf(stderr, "Cannot rename %s to %s: %s\n",
-                        MOUNTED, MOUNTED_TMP, strerror(errno));
+                DEBUG(0,("Cannot rename %s to %s: %s\n",
+			 MOUNTED, MOUNTED_TMP, strerror(errno)));
                 return;
         }
 
         if (unlink(MOUNTED"~") == -1) {
-                fprintf(stderr, "Can't remove "MOUNTED"~");
+                DEBUG(0,("Can't remove "MOUNTED"~"));
                 return;
         }
 }
@@ -298,9 +292,9 @@
 
 	while (1) {
 		if ((fd = open(mount_point, O_RDONLY)) < 0) {
-			fprintf(stderr, "mount.smbfs: can't open %s\n", mount_point);
+			DEBUG(0,("mount.smbfs: can't open %s\n", mount_point));
 			break;
-		}		
+		}
 
 		conn_options.fd = c->fd;
 		conn_options.protocol = c->protocol;
@@ -317,7 +311,8 @@
 
 		res = ioctl(fd, SMB_IOC_NEWCONN, &conn_options);
 		if (res != 0) {
-			fprintf(stderr, "mount.smbfs: ioctl failed, res=%d\n", res);
+			DEBUG(0,("mount.smbfs: ioctl failed, res=%d\n", res));
+			close(fd);
 			break;
 		}
 
@@ -332,23 +327,44 @@
 
 		close(fd);
 
-#ifndef SMBFS_DEBUG
-		/* Close all open files if we haven't done so yet. */
+		/* This looks wierd but we are only closing the userspace
+		   side, the connection has already been passed to smbfs and 
+		   it has increased the usage count on the socket.
+
+		   If we don't do this we will "leak" sockets and memory on
+		   each reconnection we have to make. */
+		cli_shutdown(c);
+		free(c);
+		c = NULL;
+
 		if (!closed) {
-			extern FILE *dbf;
+			/* redirect stdout & stderr since we can't know that
+			   the "library" functions we use are using DEBUG. */
+			if ( (fd = open("/dev/null", O_WRONLY)) < 0)
+				DEBUG(2,("mount.smbfs: can't open /dev/null\n"));
+			close_our_files(fd);
+			if (fd >= 0) {
+				dup2(fd, STDOUT_FILENO);
+				dup2(fd, STDERR_FILENO);
+				close(fd);
+			}
+
+			/* here we are no longer interactive */
+			pstrcpy(remote_machine, "smbmount");	/* sneaky ... */
+			setup_logging("mount.smbfs",False);
+			reopen_logs();
+
 			closed = 1;
-			dbf = NULL;
-			close_our_files(c?c->fd:-1);
 		}
-#endif
 
-		/* Wait for a signal from smbfs ... */
-		CatchSignal(SIGUSR1, &usr1_handler);
-		pause();
-#ifdef SMBFS_DEBUG
-		DEBUG(2,("mount.smbfs: got signal, getting new socket\n"));
-#endif
-		c = do_connection(service);
+		/* Wait for a signal from smbfs ... but don't continue
+                   until we actually get a new connection. */
+		while (!c) {
+			CatchSignal(SIGUSR1, &usr1_handler);
+			pause();
+			DEBUG(2,("mount.smbfs: got signal, getting new socket\n"));
+			c = do_connection(service);
+		}
 	}
 
 	smb_umount(mount_point);
@@ -452,10 +468,12 @@
 		fprintf(stderr,"waitpid failed: Error was %s", strerror(errno) );
 		/* FIXME: do some proper error handling */
 		exit(1);
-	}	
+	}
 
 	if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
 		fprintf(stderr,"smbmnt failed: %d\n", WEXITSTATUS(status));
+		/* FIXME: do some proper error handling */
+		exit(1);
 	}
 
 	/* Ok...  This is the rubicon for that mount point...  At any point
@@ -628,7 +646,8 @@
 	char *p;
 
 	DEBUGLEVEL = 1;
-	
+
+	/* here we are interactive, even if run from autofs */
 	setup_logging("mount.smbfs",True);
 
 	TimeInit();
@@ -654,14 +673,14 @@
 		pstrcpy(username,getenv("LOGNAME"));
 	}
 
-	parse_mount_smb(argc, argv);
-
-	DEBUG(3,("mount.smbfs started (version %s)\n", VERSION));
-
 	if (!lp_load(servicesf,True,False,False)) {
 		fprintf(stderr, "Can't load %s - run testparm to debug it\n", 
 			servicesf);
 	}
+
+	parse_mount_smb(argc, argv);
+
+	DEBUG(3,("mount.smbfs started (version %s)\n", VERSION));
 
 	codepage_initialise(lp_client_code_page());
 


More information about the samba mailing list