2.2.5pre1: unlink design flaw

[jd at epcnet.de]

Hi,

I recovered a design flaw in the unlink part of samba. If you try to re-export a smbfs-share with
samba and if you create a file with the delete-on-close flag set the file remains on disk.

Unlink over smbfs fails if the file is open.

The samba way of closing a open delete-on-close flagged file is:
 
 1. unlink file
 2. close file

But with a underlying smbfs unlink returns ETXTBUSY, so i changed it into:

  1. unlink file
  2. close file
  3. if unlink returned ETXTBUSY -> try second time

The same for an open directory which is set delete on close with kernel directory notifiys. First
the standard samba way:

  1. open dirA for notify
  2. rmdir dirA
  3. catch notify signal and close dirA

But with a underlying smbfs rmdir fails, so i changed it into:

  1. open dirA for notify
  2. rmdir dirA
  3. If rmdir fails force close of dirA notify handler und try rmdir dirA second time.

I don't know if this also apply to NTFS/FAT filesystems or any other filesystem with a non-posix
unlink and rmdir behaviour.

Diff against close.c and notify_kernel.c attached.

Greetings

      Jochen Dolze

--- close.c.orig	Mon Jun 10 13:07:45 2002
+++ close.c	Wed Jun 12 18:19:08 2002
@@ -1,4 +1,4 @@
-/* 
+/*
    Unix SMB/Netbios implementation.
    Version 1.9.
    file closing
@@ -120,6 +120,7 @@
 	share_mode_entry *share_entry = NULL;
 	size_t share_entry_count = 0;
 	BOOL delete_on_close = False;
+	BOOL delete_after_close = False;
 	connection_struct *conn = fsp->conn;
 	int err = 0;
 	int err1 = 0;
@@ -172,6 +173,8 @@
 		DEBUG(5,("close_file: file %s. Delete on close was set - deleting file.\n",
 			fsp->fsp_name));
 		if(fsp->conn->vfs_ops.unlink(conn,dos_to_unix_static(fsp->fsp_name)) != 0) {
+			if (errno == ETXTBSY) delete_after_close = True;
+
 			/*
 			 * This call can potentially fail as another smbd may have
 			 * had the file open with delete on close set and deleted
@@ -193,6 +196,23 @@
 
 	err = fd_close(conn, fsp);
 
+	if (normal_close && delete_on_close && delete_after_close) {
+		DEBUG(5,("close_file: file %s. Delete after close was set - deleting file.\n",
+			fsp->fsp_name));
+		if(fsp->conn->vfs_ops.unlink(conn,dos_to_unix_static(fsp->fsp_name)) != 0) {
+
+			/*
+			 * This call can potentially fail as another smbd may have
+			 * had the file open with delete on close set and deleted
+			 * it when its last reference to this file went away. Hence
+			 * we log this but not at debug level zero.
+			 */
+
+			DEBUG(5,("close_file: file %s. Delete after close was set and unlink failed \
+with error %s\n", fsp->fsp_name, strerror(errno) ));
+		}
+	}
+
 	/* check for magic scripts */
 	if (normal_close) {
 		check_magic(fsp,conn);
@@ -229,6 +249,9 @@
   
 static int close_directory(files_struct *fsp, BOOL normal_close)
 {
+	int delete_after_close = False;
+	BOOL ok;
+
 	remove_pending_change_notify_requests_by_fid(fsp);
 
 	/*
@@ -237,10 +260,15 @@
 	 */
 
 	if (normal_close && fsp->directory_delete_on_close) {
-		BOOL ok = rmdir_internals(fsp->conn, fsp->fsp_name);
+		ok = rmdir_internals(fsp->conn, fsp->fsp_name);
 		DEBUG(5,("close_directory: %s. Delete on close was set - deleting directory %s.\n",
 			fsp->fsp_name, ok ? "succeeded" : "failed" ));
 
+		if (!ok) {
+			delete_after_close = True;
+			errno = 0; /* reset error */
+		}
+
 		/*
 		 * Ensure we remove any change notify requests that would
 		 * now fail as the directory has been deleted.
@@ -250,10 +278,20 @@
 			remove_pending_change_notify_requests_by_filename(fsp);
 	}
 
+	if (normal_close && fsp->directory_delete_on_close && delete_after_close) {
+		remove_pending_change_notify_requests_by_filename(fsp);
+	}
+
 	/*
 	 * Do the code common to files and directories.
 	 */
 	close_filestruct(fsp);
+
+	if (normal_close && fsp->directory_delete_on_close && delete_after_close) {
+		ok = rmdir_internals(fsp->conn, fsp->fsp_name);
+		DEBUG(5,("close_directory: %s. Delete after close was set - deleting directory %s.\n", 
+			fsp->fsp_name, ok ? "succeeded" : "failed" ));
+	}
 	
 	if (fsp->fsp_name)
 		string_free(&fsp->fsp_name);
--- notify_kernel.c.orig	Sat Jun  8 14:55:58 2002
+++ notify_kernel.c	Wed Jun 12 21:25:30 2002
@@ -128,6 +128,7 @@
 		for (i = 0; i < signals_received; i++) {
 			if (fd == (int)fd_pending_array[i]) {
 				close(fd);
+				fd = -1;
 				fd_pending_array[i] = (sig_atomic_t)-1;
 				if (signals_received - i - 1) {
  					memmove((void *)&fd_pending_array[i], (void *)&fd_pending_array[i+1],
@@ -141,6 +142,8 @@
 		BlockSignals(False, RT_SIGNAL_NOTIFY);
 	}
 	SAFE_FREE(data);
+	if (fd != -1)
+		close(fd);
 	DEBUG(3,("kernel_remove_notify: fd=%d\n", fd));
 }



--- 
EPCNet GmbH
ISP & Web Design
Bleichstrasse 24
89077 Ulm

Tel.  +49 731 1416 0
Fax  +49 731 1416 120