Bug 7651 - mknod and mkfifo fails with "No such file or directory"
mknod and mkfifo fails with "No such file or directory"
Status: RESOLVED FIXED
Product: Samba 3.5
Classification: Unclassified
Component: File services
3.5.4
All Linux
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-26 06:47 UTC by Igor Zhbanov
Modified: 2010-09-06 13:45 UTC (History)
0 users

See Also:


Attachments
git am fix for 3.5.x. (1.45 KB, patch)
2010-08-26 18:52 UTC, Jeremy Allison
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Zhbanov 2010-08-26 06:47:52 UTC
Hello!

I am using samba 3.5.4. I mount Linux-server from Linux-client.
When I run mknod or mkfifo, I see that special files are created but
commands return error "No such file or directory".

The call tree is:

handle_trans2
-> call_trans2setfilepathinfo
   -> smbd_do_setfilepathinfo
      -> smb_set_file_unix_basic (The bug is here)
         -> smb_set_file_time
            -> VALID_STAT

Now look at function:
------------------------------------------------
smb_set_unix_basic function(..., const struct smb_filename *smb_fname)
{
...
	sbuf = smb_fname->st;

*** Before node was created, stat structure has zero number of links
*** since file is non-existent. And VALID_STAT checks number of links.

	if (!VALID_STAT(sbuf)) {
		struct smb_filename *smb_fname_tmp = NULL;
		/*
		 * The only valid use of this is to create character and block
		 * devices, and named pipes. This is deprecated (IMHO) and 
		 * a new info level should be used for mknod. JRA.

		status = smb_unix_mknod(conn,
					pdata,
					total_data,
					smb_fname);
		if (!NT_STATUS_IS_OK(status)) {
			return status;
		}

		status = copy_smb_filename(talloc_tos(), smb_fname,
					   &smb_fname_tmp);
		if (!NT_STATUS_IS_OK(status)) {
			return status;
		}

		if (SMB_VFS_STAT(conn, smb_fname_tmp) != 0) {
			status = map_nt_error_from_unix(errno);
			TALLOC_FREE(smb_fname_tmp);
			SMB_VFS_UNLINK(conn, smb_fname);
			return status;
		}

		sbuf = smb_fname_tmp->st;
		TALLOC_FREE(smb_fname_tmp);
		...
	}

	...

*** Few lines above we had updated (after node creation) stat structure
*** in smb_fname_tmp, but we have free()'d it...
*** So we call smb_set_file_time() with smb_fname which has non-valid
*** stat struct with zero number of links. So smb_set_file_time()
*** will return error that says "No such file or directory"
	status = smb_set_file_time(conn,
				fsp,
				smb_fname,
				&ft,
				false);
	...
}

------------------------------------------------

Here is the patch. It is so long because of const smb_fname
function argument. Perhaps you can find a better way. ;-)

diff -rup samba-3.5.4/source3/smbd/trans2.c samba-3.5.4-izh/source3/smbd/trans2.c
--- samba-3.5.4/source3/smbd/trans2.c	Fri Jun 18 16:01:04 2010
+++ samba-3.5.4-izh/source3/smbd/trans2.c	Thu Aug 26 15:24:30 2010
@@ -6595,6 +6595,9 @@ static NTSTATUS smb_set_file_unix_basic(
 	bool modify_mtime = true;
 	struct file_id id;
 	SMB_STRUCT_STAT sbuf;
+	const struct smb_filename *smb_fname_cur = smb_fname;
+	struct smb_filename *smb_fname_tmp = NULL;
+	bool node_created = false;
 
 	ZERO_STRUCT(ft);
 
@@ -6646,7 +6649,6 @@ static NTSTATUS smb_set_file_unix_basic(
 	sbuf = smb_fname->st;
 
 	if (!VALID_STAT(sbuf)) {
-		struct smb_filename *smb_fname_tmp = NULL;
 		/*
 		 * The only valid use of this is to create character and block
 		 * devices, and named pipes. This is deprecated (IMHO) and 
@@ -6675,7 +6677,8 @@ static NTSTATUS smb_set_file_unix_basic(
 		}
 
 		sbuf = smb_fname_tmp->st;
-		TALLOC_FREE(smb_fname_tmp);
+		node_created = true;
+		smb_fname_cur = smb_fname_tmp;
 
 		/* Ensure we don't try and change anything else. */
 		raw_unixmode = SMB_MODE_NO_CHANGE;
@@ -6707,8 +6710,10 @@ static NTSTATUS smb_set_file_unix_basic(
 		DEBUG(10,("smb_set_file_unix_basic: SMB_SET_FILE_UNIX_BASIC "
 			  "setting mode 0%o for file %s\n",
 			  (unsigned int)unixmode,
-			  smb_fname_str_dbg(smb_fname)));
-		if (SMB_VFS_CHMOD(conn, smb_fname->base_name, unixmode) != 0) {
+			  smb_fname_str_dbg(smb_fname_cur)));
+		if (SMB_VFS_CHMOD(conn, smb_fname_cur->base_name, unixmode) != 0) {
+			if (node_created)
+				TALLOC_FREE(smb_fname_tmp);
 			return map_nt_error_from_unix(errno);
 		}
 	}
@@ -6724,21 +6729,23 @@ static NTSTATUS smb_set_file_unix_basic(
 		DEBUG(10,("smb_set_file_unix_basic: SMB_SET_FILE_UNIX_BASIC "
 			  "changing owner %u for path %s\n",
 			  (unsigned int)set_owner,
-			  smb_fname_str_dbg(smb_fname)));
+			  smb_fname_str_dbg(smb_fname_cur)));
 
 		if (S_ISLNK(sbuf.st_ex_mode)) {
-			ret = SMB_VFS_LCHOWN(conn, smb_fname->base_name,
+			ret = SMB_VFS_LCHOWN(conn, smb_fname_cur->base_name,
 					     set_owner, (gid_t)-1);
 		} else {
-			ret = SMB_VFS_CHOWN(conn, smb_fname->base_name,
+			ret = SMB_VFS_CHOWN(conn, smb_fname_cur->base_name,
 					    set_owner, (gid_t)-1);
 		}
 
 		if (ret != 0) {
 			status = map_nt_error_from_unix(errno);
 			if (delete_on_fail) {
-				SMB_VFS_UNLINK(conn, smb_fname);
+				SMB_VFS_UNLINK(conn, smb_fname_cur);
 			}
+			if (node_created)
+				TALLOC_FREE(smb_fname_tmp);
 			return status;
 		}
 	}
@@ -6752,13 +6759,15 @@ static NTSTATUS smb_set_file_unix_basic(
 		DEBUG(10,("smb_set_file_unix_basic: SMB_SET_FILE_UNIX_BASIC "
 			  "changing group %u for file %s\n",
 			  (unsigned int)set_owner,
-			  smb_fname_str_dbg(smb_fname)));
-		if (SMB_VFS_CHOWN(conn, smb_fname->base_name, (uid_t)-1,
+			  smb_fname_str_dbg(smb_fname_cur)));
+		if (SMB_VFS_CHOWN(conn, smb_fname_cur->base_name, (uid_t)-1,
 				  set_grp) != 0) {
 			status = map_nt_error_from_unix(errno);
 			if (delete_on_fail) {
-				SMB_VFS_UNLINK(conn, smb_fname);
+				SMB_VFS_UNLINK(conn, smb_fname_cur);
 			}
+			if (node_created)
+				TALLOC_FREE(smb_fname_tmp);
 			return status;
 		}
 	}
@@ -6767,17 +6776,21 @@ static NTSTATUS smb_set_file_unix_basic(
 
 	status = smb_set_file_size(conn, req,
 				   fsp,
-				   smb_fname,
+				   smb_fname_cur,
 				   &sbuf,
 				   size,
 				   false);
 	if (!NT_STATUS_IS_OK(status)) {
+		if (node_created)
+			TALLOC_FREE(smb_fname_tmp);
 		return status;
 	}
 
 	/* Deal with any time changes. */
 	if (null_timespec(ft.mtime) && null_timespec(ft.atime)) {
 		/* No change, don't cancel anything. */
+		if (node_created)
+			TALLOC_FREE(smb_fname_tmp);
 		return status;
 	}
 
@@ -6804,13 +6817,15 @@ static NTSTATUS smb_set_file_unix_basic(
 
 	status = smb_set_file_time(conn,
 				fsp,
-				smb_fname,
+				smb_fname_cur,
 				&ft,
 				false);
 	if (modify_mtime) {
 		notify_fname(conn, NOTIFY_ACTION_MODIFIED,
-			FILE_NOTIFY_CHANGE_LAST_WRITE, smb_fname->base_name);
+			FILE_NOTIFY_CHANGE_LAST_WRITE, smb_fname_cur->base_name);
 	}
+	if (node_created)
+		TALLOC_FREE(smb_fname_tmp);
 	return status;
 }
Comment 1 Jeremy Allison 2010-08-26 18:52:01 UTC
Created attachment 5928 [details]
git am fix for 3.5.x.

Igor, please test this fix - I think it's a cleaner fix for the problem (and is what I've added into master and v3-6-test).

Thanks,

Jeremy.
Comment 2 Igor Zhbanov 2010-08-27 02:20:25 UTC
It seems that it works. :-)
Comment 3 Volker Lendecke 2010-08-27 04:44:50 UTC
Karolin, this is a patch for 3.5.

Volker
Comment 4 Karolin Seeger 2010-09-06 13:45:35 UTC
Pushed to v3-5-test.
Closing out bug report.

Thanks!