Issue with changes to VFS_STAT and VFS_LSTAT.

Jeremy Allison jra at samba.org
Fri Jun 26 16:47:34 MDT 2009


Tim,

	I need some clarifications on the changes you made in:

36c10191750c845a2a7cd6cc62149b1095c0b651..e129384d7c1df664e447186673dd107e190e2894

(s3: Plumb smb_filename through SMB_VFS_STAT and SMB_VFS_LSTAT
    
    This patch introduces two new temporary helper functions
    vfs_stat_smb_fname and vfs_lstat_smb_fname.  They basically allowed me
    to call the new smb_filename version of stat, while avoiding plumbing
    it through callers that are still too inconvenient.  As the conversion
    moves along, I will be able to remove callers of this, with the goal
    being to remove all callers.)

This pushes the 'struct smb_filename' into visibility at the
POSIX VFS layer, which as a shorthand to keep the stat struct
tied to the filename is fine.

However, in the default stat() implementation it now calls :

get_full_smb_filename() which concatinates the base name + stream
name (if one exists). Which means potentially the full stream
names are visible to an underlying POSIX system than cannot
cope with them.

Is this desired ? The logic in the upper level smbd should
ensure that basic POSIX stat is containing a stream name is never
called if the underlying filesystem can't support them.

My feeling is in the "default" module case which remember is
the last stop before hitting a *pure* "POSIX" filesystem we should
do an SMB_ASSERT() that smb_fname->stream_name == NULL, as I can't
see any case where the "default" POSIX filesystem can do
anything but error out here. It actually is exposing a logic error
in the upper level logic IMHO. A simple patch for this follows.

We also need to clarify the comments around this in all
the modules to help third-party VFS implementors to understand
*exactly* what should be going on here.

Same for the LSTAT case I think.

Can you get back to me on this and let me know what you
think ?

Jeremy.
-------------- next part --------------
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index cd792ab..1c5aa5e 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -553,21 +553,11 @@ static int vfswrap_stat(vfs_handle_struct *handle,
 			struct smb_filename *smb_fname)
 {
 	int result;
-	NTSTATUS status;
-	char *fname = NULL;
 
 	START_PROFILE(syscall_stat);
 
-	status = get_full_smb_filename(talloc_tos(), smb_fname,
-				       &fname);
-	if (!NT_STATUS_IS_OK(status)) {
-		errno = map_errno_from_nt_status(status);
-		return -1;
-	}
-
-	result = sys_stat(fname, &smb_fname->st);
-
-	TALLOC_FREE(fname);
+	SMB_ASSERT(smb_fname->stream_name == NULL);
+	result = sys_stat(smb_fname->base_name, &smb_fname->st);
 
 	END_PROFILE(syscall_stat);
 	return result;
@@ -587,21 +577,11 @@ static int vfswrap_lstat(vfs_handle_struct *handle,
 			 struct smb_filename *smb_fname)
 {
 	int result;
-	NTSTATUS status;
-	char *fname = NULL;
 
 	START_PROFILE(syscall_lstat);
 
-	status = get_full_smb_filename(talloc_tos(), smb_fname,
-				       &fname);
-	if (!NT_STATUS_IS_OK(status)) {
-		errno = map_errno_from_nt_status(status);
-		return -1;
-	}
-
-	result = sys_lstat(fname, &smb_fname->st);
-
-	TALLOC_FREE(fname);
+	SMB_ASSERT(smb_fname->stream_name == NULL);
+	result = sys_lstat(smb_fname->base_name, &smb_fname->st);
 
 	END_PROFILE(syscall_lstat);
 	return result;


More information about the samba-technical mailing list