directory change notification patch

Mark Weaver mark at npsl.co.uk
Tue Mar 22 00:53:59 GMT 2005


Replying to myself once again :)

I would appreciate any feedback on this patch.  I believe that it makes 
directory change notification much more useful and reliable than before 
when combined with kernel change notifications.  It makes IIS5 run fine 
off shares (IIS6 has been fixed to cope without reliable change 
notifications), and as an obvious side effect explorer windows now 
refresh automatically.

I would think that this probably ought to be a configuration option (off 
by default), but I've not had time for that at present (I don't require 
it).  If anyone is interested then I would be happy to do the additional 
work to get the patch included.

I have got this patch to a point that I'm pretty happy with.  It can run 
a limited set of stress tests with IIS5, explorer and a slapped together 
directory monitoring program I wrote.  These basically involve 
repeatedly changing files in the directory in a loop, and then for IIS, 
running a web stress test with a few hundred concurrent users accessing 
the same files.  It all seems to work ok so far.  More comprehensive 
testing is obviously going to involve more directories than the handful 
I used -- I will be getting to that in the next few weeks.

Notes:

1. The original patch appears to have been developed between Juergen 
Hasch  and Hal Roberts.  A google for those names should help with 
showing the development I have based my work on.

2. A number of problems from the patch by Juergen Hasch were corrected. 
  These were:
	- Truncated packets (1-3 bytes missing from the end)
	- FILE_NOTIFY_INFORMATION structures not aligned to 4 byte boundary
	- Lack of error handling
	- Empty notify packets being posted sometimes.  (I corrected this by 
reposting the directory monitoring request and ignoring the false change)
	- Wrong working directory broke the code (see #6)
	- Resources were never freed :)  Hopefully I haven't missed anything!

3. It appears that there this a problem in samba relating to signal 
handling.  I think that this is a generic problem, although for kernel 
oplocks (the other use of RT signals in SMBD that I could spot), there 
are workarounds in the code.  Essentially problems were that sys_select 
would not return EINTR sometimes, and that other code exists that uses 
sys_select_intr, which ignores EINTR.  I corrected these by changing the 
select code (see lib.patch) to use select directly in sys_select_intr, 
thus avoiding loosing a notification, and also to always return -1/EINTR 
if a signal is outstanding.  I do not know if this will break any other 
code that may rely on different behaviour (it should not, of course, but 
reality is often different :)  These two changes make signal delivery 
reliable.

4. The kernel notify code relied on reliable signal delivery (see #3). 
Since signal delivery was unreliable this led to lost notifications 
sometimes.  Although I corrected this in #3, I allowed the poll to 
continue (it seemed better to check an (almost always empty) array than 
to risk losing a signal).

5. Permissions problems, as previously mentioned.  Linux will not 
deliver DNOTIFY signals to a process if the current UID/EUID are not the 
same as those when the DNOTIFY was initiated unless the file was opened 
by root (seems like an odd choice, but it is there in the code).  This 
leads to signals being lost quite frequently.  I solved this by 
switching to root after opening the directory.  I believe that this 
takes care of any permission issues (the correct user is used to open 
the directory, but root requests the signal).  If anyone can see 
security implications here, then let me know.

6. When a notification is issued, I save the working directory.  When 
notifications are processed, I save the working directory and change to 
root.  Quite commonly the directory change processing code is run as 
root, so I can't see any security issue here.  When a notification is 
processed, it is run in the original working directory.  This ensures 
that the notification code (which stores relative paths) operates 
correctly.  Being root ensures that the directory can be reopened 
(unless it disappeared somehow) when a notification must be re-issued 
due to a false notification being detected.  There should be no issue 
with re-opening the directory as root, as the user has already had a 
permission check performed against the file.  If the notification 
completes and they re-issue it, changed permissions should then be 
picked up (as if the false notification was never issued).

7.  The patch has a memory impact -- keeping a local copy of directory 
entries in memory, as per previous changes.  For some configurations 
though, this would be worth it (and could be mitigiated by keeping the 
database on disk -- perhaps another option?).

I think that's it, although I'm happy to answer any questions.
-------------- next part --------------
diff -x *.o -x *.po -x build_options.c -x .#* -ur samba-3.0.10/source/lib/select.c samba-notify-3.0.10/source/lib/select.c
--- samba-3.0.10/source/lib/select.c	Mon Oct 25 22:04:59 2004
+++ samba-notify-3.0.10/source/lib/select.c	Mon Mar 21 21:53:32 2005
@@ -50,7 +50,7 @@
 
 /*******************************************************************
  Like select() but avoids the signal race using a pipe
- it also guuarantees that fds on return only ever contains bits set
+ it also guarantees that fds on return only ever contains bits set
  for file descriptors that were readable.
 ********************************************************************/
 
@@ -99,20 +99,17 @@
 			FD_ZERO(writefds);
 		if (errorfds)
 			FD_ZERO(errorfds);
-	}
-
-	if (FD_ISSET(select_pipe[0], readfds2)) {
+	} else if (FD_ISSET(select_pipe[0], readfds2)) {
 		char c;
 		saved_errno = errno;
 		if (read(select_pipe[0], &c, 1) == 1) {
 			pipe_read++;
-		}
-		errno = saved_errno;
-		FD_CLR(select_pipe[0], readfds2);
-		ret--;
-		if (ret == 0) {
-			ret = -1;
 			errno = EINTR;
+			ret = -1;
+		} else {
+			errno = saved_errno;
+			FD_CLR(select_pipe[0], readfds2);
+			ret--;
 		}
 	}
 
@@ -145,7 +142,18 @@
 		if (tval)
 			tval2 = *tval;
 
-		ret = sys_select(maxfd, readfds2, writefds2, errorfds2, ptval);
+		/* Changed to call select directly to avoid eating signals unnecessarily */
+		errno = 0;
+		ret = select(maxfd, readfds2, writefds2, errorfds2, ptval);
+
+		if (ret <= 0) {
+			if (readfds)
+				FD_ZERO(readfds);
+			if (writefds)
+				FD_ZERO(writefds);
+			if (errorfds)
+				FD_ZERO(errorfds);
+		}
 	} while (ret == -1 && errno == EINTR);
 
 	if (readfds)
-------------- next part --------------
diff -x *.o -x build_options.c -x .#* -ur samba-3.0.10/source/smbd/notify.c samba-notify-3.0.10/source/smbd/notify.c
--- samba-3.0.10/source/smbd/notify.c	Wed Dec 15 14:33:17 2004
+++ samba-notify-3.0.10/source/smbd/notify.c	Mon Mar 21 21:43:38 2005
@@ -24,6 +24,24 @@
 static struct cnotify_fns *cnotify;
 
 /****************************************************************************
+This structure holds a list of files and associated notification actions.
+*****************************************************************************/
+struct file_action {
+	struct file_action *next, *prev;
+	int action;
+	char *filename;
+	size_t filename_length;
+};
+
+/****************************************************************************
+This structure contains a file actions list head and element count
+*****************************************************************************/
+struct file_actions {
+	struct file_action *head;
+	uint32_t count;
+};
+
+/****************************************************************************
  This is the structure to queue to implement NT change
  notify. It consists of smb_size bytes stored from the
  transact command (to keep the mid, tid etc around).
@@ -35,17 +53,206 @@
 	files_struct *fsp;
 	connection_struct *conn;
 	uint32 flags;
+	uint32 max_parameter_count;
 	char request_buf[smb_size];
 	void *change_data;
+	char *working_directory;
+	TDB_CONTEXT *file_data;
+};
+
+/****************************************************************************
+This is the context structure used for tdb_traverse when checking for changes
+*****************************************************************************/
+struct file_data_exists_context {
+	BOOL ok;
+	struct change_notify *cnbp;
+	struct file_actions *fact;
 };
 
 static struct change_notify *change_notify_list;
 
 /****************************************************************************
- Setup the common parts of the return packet and send it.
-*****************************************************************************/
+ Free contents of a file_actions list
+****************************************************************************/
+static void change_notify_destroy_file_actions(struct file_actions *fact)
+{
+	while (fact->head != NULL) {
+		struct file_action *fa = fact->head;
+		fact->head = fact->head->next;
+		SAFE_FREE(fa->filename);
+		free(fa);
+	}
+	fact->count = 0;
+}
+
+
+/****************************************************************************
+Return a file action struct with the given filename and fileaction
+****************************************************************************/
+static struct file_action *change_notify_get_file_action(const char *filename, int fileaction)
+{
+	struct file_action *fa;
 
-static void change_notify_reply_packet(char *inbuf, NTSTATUS error_code)
+	if (!(fa = (struct file_action *)malloc(sizeof(struct file_action)))) {
+		DEBUG(0, ("change_notify_get_file_action: SMB_MALLOC failed\n"));
+		return NULL;
+	}
+	fa->action = fileaction;
+	if ( (fa->filename = SMB_STRDUP(filename)) == NULL ) {
+		DEBUG(0, ("change_notify_get_file_action: strdup failed\n"));
+		free(fa);
+		return NULL;
+	}
+	fa->filename_length = strlen(filename);
+
+	return fa;
+}
+
+/****************************************************************************
+Check to make sure that the file in the given cnbp.file_data record
+still exists.
+****************************************************************************/
+static int change_notify_file_data_exists(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA data, void *state)
+{
+
+	struct file_action *fa;
+	struct file_data_exists_context *context = (struct file_data_exists_context *)state;
+	char *filename;
+	const char *path = context->cnbp->fsp->fsp_name;
+	size_t filename_len = strlen((char *)key.dptr) + strlen((char *)path) + 2;
+	int fd;
+
+	/* Make a full path to the file */
+	if (!(filename = (char *)SMB_MALLOC(filename_len))) {
+		DEBUG(0, ("change_notify_file_data_exists: malloc failed\n"));
+		context->ok = False;
+		return 1; /* stop traversing */
+
+	}
+
+	filename[0] = '\0';
+	safe_strcat(filename, (char *)path, filename_len);
+	safe_strcat(filename, "/", filename_len);
+	safe_strcat(filename, (char *)key.dptr, filename_len);
+
+	/* Try and open it */
+	if ((fd = open(filename, O_RDONLY)) == -1) {
+		fa = change_notify_get_file_action
+			((char *)key.dptr, FILE_ACTION_REMOVED);
+		if (fa == NULL) {
+			free(filename);
+			DEBUG(0, ("change_notify_file_data_exists: getting file action failed\n"));
+			context->ok = False;
+			return 1; /* stop traversing */
+		}
+		++context->fact->count;
+		DLIST_ADD(context->fact->head, fa);
+	} else {
+		close(fd);
+	}
+	free(filename);
+
+	return 0;
+}
+
+/****************************************************************************
+Get a list of all the file notification actions
+****************************************************************************/
+static BOOL change_notify_find_file_actions(struct change_notify *cnbp, struct file_actions *fact)
+{
+	void *dp;
+	const char *fname;
+	TDB_DATA tdb_key, tdb_data;
+	SMB_STRUCT_STAT old_st, new_st;
+	pstring full_name;
+	size_t fullname_len, remaining_len;
+	char *p;
+	struct file_action *fa;
+	struct file_data_exists_context exists_context;
+
+	/* Initialise the output list */
+	fact->head = NULL;
+	fact->count = 0;
+
+	if (!(dp = OpenDir(cnbp->conn, cnbp->fsp->fsp_name, True))) {
+		DEBUG(0, ("change_notify_find_file_actions: failed to open directory '%s' with %d, uid = %d, euid=%d\n",
+			  cnbp->fsp->fsp_name, errno, getuid(), geteuid()));
+		goto err;
+	}
+
+	pstrcpy(full_name, cnbp->fsp->fsp_name);
+	pstrcat(full_name, "/");
+
+	fullname_len = strlen(full_name);
+	remaining_len = sizeof(full_name) - fullname_len - 1;
+	p = &full_name[fullname_len];
+
+	while ((fname = ReadDirName(dp))) {
+		if (strequal(fname, ".") || strequal(fname, "..")) {
+			continue;
+		}
+
+		safe_strcpy(p, fname, remaining_len);
+		if (SMB_VFS_STAT(cnbp->conn, full_name, &old_st) != 0) {
+			DEBUG(10,("change_notify_find_file_actions: stat failed for '%s', ignoring the file\n",
+				  full_name));
+		} else {
+			tdb_key.dptr = (void *)fname;
+			tdb_key.dsize = strlen(fname) + 1;
+			tdb_data = tdb_fetch(cnbp->file_data, tdb_key);
+			if (!tdb_data.dptr) {
+				fa = change_notify_get_file_action
+					(fname, FILE_ACTION_ADDED);
+				if (fa == NULL)
+					goto err;
+				++fact->count;
+				DLIST_ADD(fact->head, fa);
+				continue;
+			}
+			new_st = *((SMB_STRUCT_STAT *)tdb_data.dptr);
+			free(tdb_data.dptr);
+			if ((old_st.st_atime != new_st.st_atime) ||
+		    	(old_st.st_mtime != new_st.st_mtime) ||
+			    (old_st.st_ctime != new_st.st_ctime)) {
+				fa = change_notify_get_file_action
+					(fname, FILE_ACTION_MODIFIED);
+				if (fa == NULL)
+					goto err;
+				++fact->count;
+				DLIST_ADD(fact->head, fa);
+			}
+		}
+	}
+	CloseDir(dp);
+
+	/* check for deleted files */
+	exists_context.fact = fact;
+	exists_context.cnbp = cnbp;
+	exists_context.ok = True;
+	if (tdb_traverse(cnbp->file_data, &change_notify_file_data_exists, &exists_context) == -1) {
+		DEBUG(0, ("change_notify_find_file_actions: traverse failed: %s\n", tdb_errorstr(cnbp->file_data)));
+		goto err;
+	}
+	if (!exists_context.ok) {
+		DEBUG(0, ("change_notify_find_file_actions: traverse encountered a problem\n"));
+		goto err;
+	}
+
+	return True;
+
+ err:
+	change_notify_destroy_file_actions(fact);
+	return False;
+}
+
+/****************************************************************************
+ * Send an error code back
+ *
+ * @param inbuf
+ * @param error_code NTSTATUS value
+ *
+ *****************************************************************************/
+static void change_notify_reply_error(char *inbuf, NTSTATUS error_code)
 {
 	char outbuf[smb_size+38];
 
@@ -61,9 +268,115 @@
 	set_message(outbuf,18,0,False);
 
 	if (!send_smb(smbd_server_fd(),outbuf))
+		exit_server("change_notify_reply_error: send_smb failed.");
+}
+
+/****************************************************************************
+ * Setup a reply packet with the names of individual files changed and send it
+ *
+ * Returns True if a reply was sent, False if not.  In the False case, this
+ * means that their was no information to send (packets are always sent if
+ * an error has occurred; currently these contain STATUS_NOTIFY_ENUM_DIR as
+ * I'm not sure which error code(s) would be appropriate to send).  This at
+ * least will inform the application monitoring that something has happened,
+ * and presumably it will find out later what went wrong (if it posts another
+ * change notify, for example).
+ *****************************************************************************/
+static BOOL change_notify_reply_packet(struct change_notify *cnbp)
+{
+	struct file_action *fa;
+	char *outbuf;
+	char *p;
+	int offset;
+	uint32_t total;
+	BOOL rc;
+	int alignment = 4- smb_size%4;
+	struct file_actions fact;
+
+	/* Get changes */
+	if (!change_notify_find_file_actions(cnbp, &fact)) {
+		DEBUG(10, ("change_notify_find_file_actions failed: returning STATUS_NOTIFY_ENUM_DIR\n"));
+		goto err_enum_dir;
+	}
+
+	/* Check if any changes were noted.  If not, return False which
+       will cause the caller to monitor the directory again
+	 */
+	if (fact.count == 0) {
+		change_notify_destroy_file_actions(&fact);
+		return False;
+	}
+
+
+	/* return STATUS_NOTIFY_ENUM_DIR if we have too many files or none at all  */
+	if (fact.count >= cnbp->max_parameter_count) {
+		DEBUG(10,("change_notify_reply_packet: returning STATUS_NOTIFY_ENUM_DIR with max parameter count %u, count %u\n", cnbp->max_parameter_count, fact.count));
+		goto err_enum_dir;
+	}
+
+
+	/* calculate parameter size to get max packet length */
+	for (total = 0, fa = fact.head; fa; fa = fa->next) {
+		size_t length = strlen(fa->filename) * 2 + 12;
+		/* align FILE_NOTIFY_INFORMATION to 4 byte boundary */
+		length += (length%4 ? 4-(length%4) : 0);
+		total += length;
+	}
+	total += smb_size+ 2*18+alignment;
+
+	if (total > 65535) {
+		DEBUG(10,("change_notify_reply_packet: returning STATUS_NOTIFY_ENUM_DIR with max parameter count %u, count %u, total %u\n", cnbp->max_parameter_count, fact.count, total));
+		goto err_enum_dir;
+	}
+	outbuf = SMB_MALLOC(total);
+	if (outbuf == NULL) {
+		DEBUG(0,("change_notify_reply_packet: SMB_MALLOC failed for %u bytes, returning STATUS_NOTIFY_ENUM_DIR\n", total));
+		goto err_enum_dir;
+	}
+
+	/* setup reply packet */
+	memset(outbuf, '\0', total);
+	construct_reply_common(cnbp->request_buf, outbuf);
+	set_message(outbuf,18,0,False);
+
+	/* fill NT NOTIFY parameters */
+	p = smb_buf(outbuf) + alignment;
+	for (total = 0, fa = fact.head; fa; fa = fa->next) {
+		/* Set the filename, byte length and action fields */
+		size_t length = push_ucs2(NULL, p+12, fa->filename, strlen(fa->filename)*2, 0);
+		DEBUG(10,("change_notify_reply_packet: file %s, action = %d\n", fa->filename, fa->action));
+		SIVAL(p,8,length);
+		SIVAL(p,4,fa->action);
+		offset = length +12;
+		/* align FNI structure to 4 byte boundary */
+		offset += (length%4 ? 4-(length%4) : 0);
+		/* Set offset to next structure (0 for last structure */
+		SIVAL(p,0, (fa->next ? offset : 0));
+		p += offset;
+		total += offset;
+	}
+	SIVAL(outbuf,smb_ntr_TotalParameterCount,total);
+	SIVAL(outbuf,smb_ntr_ParameterCount,total);
+	set_message(outbuf,18,total + alignment,False);
+	SIVAL(outbuf,smb_ntr_ParameterOffset,smb_buf(outbuf) + alignment - smb_base(outbuf));
+
+	/* done with the notifications list */
+	change_notify_destroy_file_actions(&fact);
+
+	/* send the response */
+	rc = send_smb(smbd_server_fd(),outbuf);
+	free(outbuf);
+	if (!rc)
 		exit_server("change_notify_reply_packet: send_smb failed.");
+	return True;
+
+ err_enum_dir:
+	change_notify_destroy_file_actions(&fact);
+	change_notify_reply_error(cnbp->request_buf, STATUS_NOTIFY_ENUM_DIR);
+	return True;
 }
 
+
 /****************************************************************************
  Remove an entry from the list and free it, also closing any
  directory handle if necessary.
@@ -72,11 +385,29 @@
 static void change_notify_remove(struct change_notify *cnbp)
 {
 	cnotify->remove_notify(cnbp->change_data);
+	if (cnbp->file_data)
+		tdb_close(cnbp->file_data);
+	SAFE_FREE(cnbp->working_directory);
 	DLIST_REMOVE(change_notify_list, cnbp);
 	ZERO_STRUCTP(cnbp);
 	SAFE_FREE(cnbp);
 }
 
+				
+static BOOL change_notify_restart(struct change_notify *cnbp)
+{
+	/* Remove the old notification if required */
+	cnotify->remove_notify(cnbp->change_data);
+	/* Register another one */
+	cnbp->change_data = cnotify->register_notify(cnbp->conn, cnbp->fsp->fsp_name, cnbp->flags);
+	if (!cnbp->change_data) {
+		DEBUG(0,("change_notify_restart: register_notify failed for '%s'\n", cnbp->fsp->fsp_name));
+		return False;
+	}
+	return True;
+}
+
+
 /****************************************************************************
  Delete entries by fnum from the change notify pending queue.
 *****************************************************************************/
@@ -104,7 +435,7 @@
 	for (cnbp=change_notify_list; cnbp; cnbp=next) {
 		next=cnbp->next;
 		if(SVAL(cnbp->request_buf,smb_mid) == mid) {
-			change_notify_reply_packet(cnbp->request_buf,NT_STATUS_CANCELLED);
+			change_notify_reply_error(cnbp->request_buf,NT_STATUS_CANCELLED);
 			change_notify_remove(cnbp);
 		}
 	}
@@ -126,7 +457,7 @@
 		 * the filename are identical.
 		 */
 		if((cnbp->fsp->conn == fsp->conn) && strequal(cnbp->fsp->fsp_name,fsp->fsp_name)) {
-			change_notify_reply_packet(cnbp->request_buf,NT_STATUS_CANCELLED);
+			change_notify_reply_error(cnbp->request_buf,NT_STATUS_CANCELLED);
 			change_notify_remove(cnbp);
 		}
 	}
@@ -151,47 +482,166 @@
 {
 	struct change_notify *cnbp, *next;
 	uint16 vuid;
+	pstring wd;
 
+	/* Store the current working directory, in case needed */
+	if (sys_getwd(wd) == NULL) {
+		DEBUG(0,("process_pending_change_notify_queue: sys_getwd failed\n"));
+		return False;
+	}
+
+	/* TODO: better to switch to vuid? Most of the time, this runs as root
+	   anyway
+     */
+	become_root();
 	for (cnbp=change_notify_list; cnbp; cnbp=next) {
 		next=cnbp->next;
 
 		vuid = (lp_security() == SEC_SHARE) ? UID_FIELD_INVALID : SVAL(cnbp->request_buf,smb_uid);
 
+		DEBUG(10,("process_pending_change_notify_queue: checking notify for %s/%s\n", cnbp->working_directory, cnbp->fsp->fsp_name));
 		if (cnotify->check_notify(cnbp->conn, vuid, cnbp->fsp->fsp_name, cnbp->flags, cnbp->change_data, t)) {
-			DEBUG(10,("process_pending_change_notify_queue: dir %s changed !\n", cnbp->fsp->fsp_name ));
-			change_notify_reply_packet(cnbp->request_buf,STATUS_NOTIFY_ENUM_DIR);
-			change_notify_remove(cnbp);
+			/* change to the appropriate directory first */
+			chdir(cnbp->working_directory);
+
+			DEBUG(10,("process_pending_change_notify_queue: directory '%s' changed\n", cnbp->fsp->fsp_name ));
+			/* See if anything really changed */
+			if (!change_notify_reply_packet(cnbp)) {
+				DEBUG(10,("process_pending_change_notify_queue: directory '%s' didn't really change, queuing another notification\n", cnbp->fsp->fsp_name));
+				/* If not, restart the notification, but don't send a reply packet */
+				if (!change_notify_restart(cnbp)) {
+					/* Restart failed, so give a `stuff changed` error */
+					change_notify_reply_error(cnbp->request_buf, STATUS_NOTIFY_ENUM_DIR);
+					change_notify_remove(cnbp);
+				}
+			} else {
+				/* Sent a packet so remove the old notification */
+			    change_notify_remove(cnbp);
+			}
 		}
 	}
+	/* Restore old user/directory */
+	unbecome_root();
+	if (chdir(wd) != 0) {
+		DEBUG(0,("process_pending_change_notify_queue: failed to change back to directory '%s'\n",
+				 wd));
+	}
 
 	return (change_notify_list != NULL);
 }
 
 /****************************************************************************
+ Setup the file_data field in the change_notify struct to contain
+ a tdb table of file stat data keyed by file name
+****************************************************************************/
+static BOOL change_notify_setup_file_data(struct change_notify *cnbp) {
+
+	void *dp;
+	const char *fname;
+	TDB_DATA tdb_key, tdb_data;
+	SMB_STRUCT_STAT st;
+	pstring full_name;
+	size_t fullname_len, remaining_len;
+	char *p;
+
+	if (!(cnbp->file_data = tdb_open("/dev/null", 0, TDB_INTERNAL,
+					 O_RDWR | O_CREAT | O_TRUNC, 0600))) {
+		DEBUG(0, ("change_notify_setup_file_data: failed to open file data db\n"));
+		return False;
+	}
+
+	if (!(dp = OpenDir(cnbp->conn, cnbp->fsp->fsp_name, True))) {
+		DEBUG(0, ("change_notify_setup_file_data: failed to open directory '%s'\n", cnbp->fsp->fsp_name));
+		tdb_close(cnbp->file_data);
+		cnbp->file_data = NULL;
+		return False;
+	}
+
+
+	pstrcpy(full_name, cnbp->fsp->fsp_name);
+	pstrcat(full_name, "/");
+
+	fullname_len = strlen(full_name);
+	remaining_len = sizeof(full_name) - fullname_len - 1;
+	p = &full_name[fullname_len];
+
+	while ((fname = ReadDirName(dp))) {
+		if (strequal(fname, ".") || strequal(fname, "..")) {
+			continue;
+		}
+
+		safe_strcpy(p, fname, remaining_len);
+
+		if (SMB_VFS_STAT(cnbp->conn, full_name, &st) != 0) {
+			DEBUG(10, ("change_notify_setup_file_data: stat failed for '%s', ignoring\n", full_name));
+			continue;
+		}
+
+		tdb_key.dptr = (void *)fname;
+		tdb_key.dsize = strlen(fname) + 1;
+
+		tdb_data.dptr = (void *)&st;
+		tdb_data.dsize = (size_t)sizeof(st);
+
+		if (tdb_store(cnbp->file_data, tdb_key, tdb_data, 0)) {
+			DEBUG(0, ("change_notify_setup_file_data: unable to store data for '%s': %s\n", full_name, tdb_errorstr(cnbp->file_data)));
+			tdb_close(cnbp->file_data);
+			cnbp->file_data = NULL;
+			CloseDir(dp);
+			return False;
+		}
+	}
+	CloseDir(dp);
+	return True;
+}
+
+/****************************************************************************
  Now queue an entry on the notify change list.
  We only need to save smb_size bytes from this incoming packet
  as we will always by returning a 'read the directory yourself'
  error.
 ****************************************************************************/
 
-BOOL change_notify_set(char *inbuf, files_struct *fsp, connection_struct *conn, uint32 flags)
+BOOL change_notify_set(char *inbuf, files_struct *fsp, connection_struct *conn, uint32 flags, uint32 max_parameter_count)
 {
 	struct change_notify *cnbp;
+	pstring wd;
 
+	DEBUG(10,("change_notify_set called\n"));
 	if((cnbp = SMB_MALLOC_P(struct change_notify)) == NULL) {
 		DEBUG(0,("change_notify_set: malloc fail !\n" ));
-		return -1;
+		return False;
 	}
 
 	ZERO_STRUCTP(cnbp);
 
+	/* Store the working directory for operating on the fsp sensibly */
+	if (!sys_getwd(wd)) {
+		DEBUG(0,("change_notify_set: sys_getwd failed\n"));
+		SAFE_FREE(cnbp);
+		return False;
+	}
+	if ( (cnbp->working_directory = SMB_STRDUP(wd)) == NULL ) {
+		DEBUG(0,("change_notify_set: strdup failed\n"));
+		SAFE_FREE(cnbp);
+		return False;
+	}
+
 	memcpy(cnbp->request_buf, inbuf, smb_size);
 	cnbp->fsp = fsp;
 	cnbp->conn = conn;
 	cnbp->flags = flags;
+	cnbp->max_parameter_count = max_parameter_count;
 	cnbp->change_data = cnotify->register_notify(conn, fsp->fsp_name, flags);
 	
 	if (!cnbp->change_data) {
+		DEBUG(10,("change_notify_set: register_notify failed for '%s'\n", fsp->fsp_name));
+		SAFE_FREE(cnbp);
+		return False;
+	}
+
+	if (!change_notify_setup_file_data(cnbp)) {
+		cnotify->remove_notify(cnbp->change_data);
 		SAFE_FREE(cnbp);
 		return False;
 	}
diff -x *.o -x build_options.c -x .#* -ur samba-3.0.10/source/smbd/notify_kernel.c samba-notify-3.0.10/source/smbd/notify_kernel.c
--- samba-3.0.10/source/smbd/notify_kernel.c	Mon Oct 25 22:04:54 2004
+++ samba-notify-3.0.10/source/smbd/notify_kernel.c	Mon Mar 21 21:59:38 2005
@@ -23,7 +23,7 @@
 
 #if HAVE_KERNEL_CHANGE_NOTIFY
 
-#define FD_PENDING_SIZE 20
+#define FD_PENDING_SIZE 10000
 static SIG_ATOMIC_T fd_pending_array[FD_PENDING_SIZE];
 static SIG_ATOMIC_T signals_received;
 
@@ -89,8 +89,17 @@
 	int i;
 	BOOL ret = False;
 
-	if (t)
-		return False;
+	/* TODO: took this out as signals were not always caught,
+		due to e.g. calling sys_select_intr.  Although that ought
+		to be fixed, the polling method is infrequent, and most of
+		the time there are no waiting signals.
+		However, performing the polling provides an opportunity
+		to recover in the case of lost signals, which is better
+		than breaking.
+	*/
+	DEBUG(10,("kernel_check_notify at %u, fd=%d, signals=%d\n", t, data->directory_handle, signals_received));
+	/*	if (t)
+		return False;*/
 
 	BlockSignals(True, RT_SIGNAL_NOTIFY);
 	for (i = 0; i < signals_received; i++) {
@@ -150,20 +159,31 @@
 
 static void *kernel_register_notify(connection_struct *conn, char *path, uint32 flags)
 {
-	struct change_data data;
-	int fd;
+	struct change_data *data = NULL;
+	int uid_switched = 0;
 	unsigned long kernel_flags;
 	
-	fd = sys_open(path,O_RDONLY, 0);
+	data = SMB_MALLOC(sizeof(struct change_data));
+	if (data == NULL) {
+		DEBUG(3,("Failed to allocate memory for a change notification\n"));
+		goto err;
+	}
 
-	if (fd == -1) {
+	data->directory_handle = sys_open(path,O_RDONLY, 0);
+	if (data->directory_handle == -1) {
 		DEBUG(3,("Failed to open directory %s for change notify\n", path));
-		return NULL;
+		goto err;
 	}
 
-	if (sys_fcntl_long(fd, F_SETSIG, RT_SIGNAL_NOTIFY) == -1) {
+	/* Become root so that signals are always delivered for this fd.
+	   Security should have been checked with the sys_open above
+     */
+	become_root();
+	uid_switched = 1;
+
+	if (sys_fcntl_long(data->directory_handle, F_SETSIG, RT_SIGNAL_NOTIFY) == -1) {
 		DEBUG(3,("Failed to set signal handler for change notify\n"));
-		return NULL;
+		goto err;
 	}
 
 	kernel_flags = DN_CREATE|DN_DELETE|DN_RENAME; /* creation/deletion changes everything! */
@@ -177,18 +197,28 @@
 	if (flags & FILE_NOTIFY_CHANGE_SECURITY)    kernel_flags |= DN_ATTRIB;
 	if (flags & FILE_NOTIFY_CHANGE_EA)          kernel_flags |= DN_ATTRIB;
 	if (flags & FILE_NOTIFY_CHANGE_FILE_NAME)   kernel_flags |= DN_RENAME|DN_DELETE;
-
-	if (sys_fcntl_long(fd, F_NOTIFY, kernel_flags) == -1) {
+	if (sys_fcntl_long(data->directory_handle, F_NOTIFY, kernel_flags) == -1) {
 		DEBUG(3,("Failed to set async flag for change notify\n"));
-		return NULL;
+		goto err;
 	}
-
-	data.directory_handle = fd;
+	unbecome_root();
 
 	DEBUG(3,("kernel change notify on %s (ntflags=0x%x flags=0x%x) fd=%d\n", 
-		 path, (int)flags, (int)kernel_flags, fd));
+		 path, (int)flags, (int)kernel_flags, data->directory_handle));
+	return data;
 
-	return (void *)memdup(&data, sizeof(data));
+err:
+	/* Change user back */
+	if (uid_switched) {
+		unbecome_root();
+	}
+	/* Free data/fds */
+	if (data) {
+		if (data->directory_handle != -1)
+			close(data->directory_handle);
+		free(data);
+	}
+	return NULL;
 }
 
 /****************************************************************************
diff -x *.o -x build_options.c -x .#* -ur samba-3.0.10/source/smbd/nttrans.c samba-notify-3.0.10/source/smbd/nttrans.c
--- samba-3.0.10/source/smbd/nttrans.c	Wed Dec 15 14:33:17 2004
+++ samba-notify-3.0.10/source/smbd/nttrans.c	Wed Mar 16 17:30:54 2005
@@ -1788,6 +1788,7 @@
 	char *setup = *ppsetup;
 	files_struct *fsp;
 	uint32 flags;
+	uint32 max_parameter_count = IVAL(inbuf, smb_nt_MaxParameterCount);
 
         if(setup_count < 6)
 		return ERROR_DOS(ERRDOS,ERRbadfunc);
@@ -1803,7 +1804,7 @@
 	if((!fsp->is_directory) || (conn != fsp->conn))
 		return ERROR_DOS(ERRDOS,ERRbadfid);
 
-	if (!change_notify_set(inbuf, fsp, conn, flags))
+	if (!change_notify_set(inbuf, fsp, conn, flags, max_parameter_count))
 		return(UNIXERROR(ERRDOS,ERRbadfid));
 
 	DEBUG(3,("call_nt_transact_notify_change: notify change called on directory \
diff -x *.o -x build_options.c -x .#* -ur samba-3.0.10/source/smbd/process.c samba-notify-3.0.10/source/smbd/process.c
--- samba-3.0.10/source/smbd/process.c	Wed Dec 15 14:33:17 2004
+++ samba-notify-3.0.10/source/smbd/process.c	Mon Mar 21 19:15:51 2005
@@ -430,6 +430,7 @@
 	FD_SET(smbd_server_fd(),&fds);
 	maxfd = setup_oplock_select_set(&fds);
 
+	DEBUG(10,("calling sys_select\n"));
 	selrtn = sys_select(MAX(maxfd,smbd_server_fd())+1,&fds,NULL,NULL,pto);
 
 	/* if we get EINTR then maybe we have received an oplock
@@ -437,6 +438,7 @@
 	   is the best we can do until the oplock code knows more about
 	   signals */
 	if (selrtn == -1 && errno == EINTR) {
+		DEBUG(10,("async processing time\n"));
 		async_processing(buffer, buffer_len);
 		/*
 		 * After async processing we must go and do the select again, as
@@ -1593,7 +1595,7 @@
 
 		if ((num_smbs % 50) == 0 && need_to_check_log_size()) {
 			change_to_root_user();
-			check_log_size();
+			check_log_size();	
 		}
 	}
 }
-------------- next part --------------
diff -x stamp-h -x wrepld_proto.h -x build_env.h -x config.h -x config.h.in~ -x proto.h samba-3.0.10/source/include/smb.h samba-notify-3.0.10/source/include/smb.h
1201a1202,1211
> /* file notification actions */
> #define FILE_ACTION_ADDED              0x00000001
> #define FILE_ACTION_REMOVED            0x00000002
> #define FILE_ACTION_MODIFIED		   0x00000003
> #define FILE_ACTION_RENAMED_OLD_NAME   0x00000004
> #define FILE_ACTION_RENAMED_NEW_NAME   0x00000005
> #define FILE_ACTION_ADDED_STREAM	   0x00000006
> #define FILE_ACTION_REMOVED_STREAM	   0x00000007
> #define FILE_ACTION_MODIFIED_STREAM    0x00000008
> 


More information about the samba-technical mailing list