[PATCH] Start moving lp_posix_pathnames out of utility functions.

Ralph Boehme rb at sernet.de
Sat Dec 12 21:46:01 UTC 2015


On Fri, Dec 11, 2015 at 03:07:30PM -0800, Jeremy Allison wrote:
> From the "I broke it so I should help fix it" department :-).
> 
> Here is a patchset that ends up removing lp_posix_pathnames()
> from ms_has_wild(), where it had no business being.
> 
> If we're ever to make SMB2 unix extensions a reality
> we need to have a handle-based posix pathname flag
> for SMB2 processing, not grubbing around with a global
> hidden behind a function (that can be left for the SMB1
> code :-).
> 
> Ultimate goal is to add a 'bool posix_paths' flag
> to the struct smbd_smb2_request struct.
> 
> Volker, let me know if this works for you ?
> 
> Please review and push if you're happy.

lgtm!

While we're at it, maybe we can start splitting out POSIX pathnames
capability out of file_struct.posix_open.FSP_POSIX_FLAGS_OPEN and add
FSP_POSIX_FLAGS_PATHNAMES, please see attached patchset.

Btw, it seems we somehow messed up the POSIX rename patchset and now
open_file_ntcreate() doesn't set FSP_POSIX_FLAGS_RENAME when
posix_open is true. I'll take a closer look tomorrow.

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From 92142c41ce28b2e3a27fa06ab13083a4a2e35645 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:33:22 -0800
Subject: [PATCH 01/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Ensure we set posix_pathnames early.

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/filename.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 555658d..4b40df3 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -229,7 +229,8 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 	char *stream = NULL;
 	bool component_was_mangled = False;
 	bool name_has_wildcard = False;
-	bool posix_pathnames = false;
+	bool posix_pathnames = (lp_posix_pathnames() ||
+				(ucf_flags & UCF_POSIX_PATHNAMES));
 	bool allow_wcard_last_component =
 	    (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP);
 	bool save_last_component = ucf_flags & UCF_SAVE_LCOMP;
@@ -348,9 +349,6 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 		}
 	}
 
-	posix_pathnames = (lp_posix_pathnames() ||
-				(ucf_flags & UCF_POSIX_PATHNAMES));
-
 	/*
 	 * Strip off the stream, and add it back when we're done with the
 	 * base_name.
-- 
2.5.0


From 3330bf38930932162ca4a22fe980f22a86b7829b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:36:33 -0800
Subject: [PATCH 02/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fix determine_path_error().

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/filename.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 4b40df3..826162b 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -55,9 +55,11 @@ static bool mangled_equal(const char *name1,
 ****************************************************************************/
 
 static NTSTATUS determine_path_error(const char *name,
-			bool allow_wcard_last_component)
+			bool allow_wcard_last_component,
+			bool posix_pathnames)
 {
 	const char *p;
+	bool name_has_wild = false;
 
 	if (!allow_wcard_last_component) {
 		/* Error code within a pathname. */
@@ -74,7 +76,11 @@ static NTSTATUS determine_path_error(const char *name,
 
 	p = strchr(name, '/');
 
-	if (!p && (ms_has_wild(name) || ISDOT(name))) {
+	if (!posix_pathnames) {
+		name_has_wild = ms_has_wild(name);
+	}
+
+	if (!p && (name_has_wild || ISDOT(name))) {
 		/* Error code at the end of a pathname. */
 		return NT_STATUS_OBJECT_NAME_INVALID;
 	} else {
@@ -300,7 +306,8 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 			status = NT_STATUS_OBJECT_NAME_INVALID;
 		} else {
 			status =determine_path_error(&orig_path[2],
-			    allow_wcard_last_component);
+			    allow_wcard_last_component,
+			    posix_pathnames);
 		}
 		goto err;
 	}
@@ -626,7 +633,8 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 				status = NT_STATUS_OBJECT_NAME_INVALID;
 			} else {
 				status = determine_path_error(end+1,
-						allow_wcard_last_component);
+						allow_wcard_last_component,
+						posix_pathnames);
 			}
 			goto fail;
 		}
-- 
2.5.0


From 24bd4a0b3710a22591c16c94c817ac127af78174 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:38:49 -0800
Subject: [PATCH 03/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fixup check_parent_exists().

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/filename.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 826162b..39b351b 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -128,6 +128,7 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
 	const char *last_component = NULL;
 	NTSTATUS status;
 	int ret;
+	bool parent_fname_has_wild = false;
 
 	ZERO_STRUCT(parent_fname);
 	if (!parent_dirname(ctx, smb_fname->base_name,
@@ -136,6 +137,10 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
+	if (!posix_pathnames) {
+		parent_fname_has_wild = ms_has_wild(parent_fname.base_name);
+	}
+
 	/*
 	 * If there was no parent component in
 	 * smb_fname->base_name of the parent name
@@ -143,7 +148,7 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
 	 * optimization.
 	 */
 	if ((smb_fname->base_name == last_component) ||
-			ms_has_wild(parent_fname.base_name)) {
+			parent_fname_has_wild) {
 		return NT_STATUS_OK;
 	}
 
-- 
2.5.0


From 7b956756e62e9cc43d3b2410392a4cf7d3550f53 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:41:38 -0800
Subject: [PATCH 04/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fixup unix_convert().

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/filename.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 39b351b..1174b59 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -448,11 +448,14 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 	 * is true.
 	 */
 
-	name_has_wildcard = ms_has_wild(smb_fname->base_name);
-	if (name_has_wildcard && !allow_wcard_last_component) {
-		/* Wildcard not valid anywhere. */
-		status = NT_STATUS_OBJECT_NAME_INVALID;
-		goto fail;
+	if (!posix_pathnames) {
+		/* POSIX pathnames have no wildcards. */
+		name_has_wildcard = ms_has_wild(smb_fname->base_name);
+		if (name_has_wildcard && !allow_wcard_last_component) {
+			/* Wildcard not valid anywhere. */
+			status = NT_STATUS_OBJECT_NAME_INVALID;
+			goto fail;
+		}
 	}
 
 	DEBUG(5,("unix_convert begin: name = %s, dirpath = %s, start = %s\n",
@@ -647,7 +650,9 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
 		/* The name cannot have a wildcard if it's not
 		   the last component. */
 
-		name_has_wildcard = ms_has_wild(start);
+		if (!posix_pathnames) {
+			name_has_wildcard = ms_has_wild(start);
+		}
 
 		/* Wildcards never valid within a pathname. */
 		if (name_has_wildcard && end) {
-- 
2.5.0


From 0632390d340e9deec66d2781c1b74514888195ee Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:45:37 -0800
Subject: [PATCH 05/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fixup reply_ntrename().

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/nttrans.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 19c7153..098d88b 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -1548,7 +1548,7 @@ void reply_ntrename(struct smb_request *req)
 		goto out;
 	}
 
-	if (ms_has_wild(oldname)) {
+	if (!lp_posix_pathnames() && ms_has_wild(oldname)) {
 		reply_nterror(req, NT_STATUS_OBJECT_PATH_SYNTAX_BAD);
 		goto out;
 	}
-- 
2.5.0


From cb68d2f065f03fda4dbaa0e958ab87d7c6b5fa38 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 12 Dec 2015 22:31:24 +0100
Subject: [PATCH 06/10] smbd: file_struct: factor out POSIX pathname processing
 out of POSIX open

Factor out another POSIX cabability from file_struct.posix_flags
FSP_POSIX_FLAGS_OPEN and start using it. Places that still use
FSP_POSIX_FLAGS_OPEN when dealing with pathnames can be converted later,
because we still set the kitchen sink flag FSP_POSIX_FLAGS_OPEN in fsp.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/include/vfs.h | 2 ++
 source3/smbd/open.c   | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 17bd8fa..71e1af9 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -301,9 +301,11 @@ typedef struct files_struct {
 
 #define FSP_POSIX_FLAGS_OPEN		0x01
 #define FSP_POSIX_FLAGS_RENAME		0x02
+#define FSP_POSIX_FLAGS_PATHNAMES	0x04
 
 #define FSP_POSIX_FLAGS_ALL			\
 	(FSP_POSIX_FLAGS_OPEN |			\
+	 FSP_POSIX_FLAGS_PATHNAMES |		\
 	 FSP_POSIX_FLAGS_RENAME)
 
 struct vuid_cache_entry {
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 9cd415b..4071f0f 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2703,7 +2703,9 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	fsp->access_mask = open_access_mask; /* We change this to the
 					      * requested access_mask after
 					      * the open is done. */
-	fsp->posix_flags |= posix_open ? FSP_POSIX_FLAGS_OPEN : 0;
+	if (posix_open) {
+		fsp->posix_flags |= FSP_POSIX_FLAGS_OPEN | FSP_POSIX_FLAGS_PATHNAMES;
+	}
 
 	if (timeval_is_zero(&request_time)) {
 		request_time = fsp->open_time;
-- 
2.5.0


From 616663cb0c1601e28e24720ed069508022adf7d3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:49:44 -0800
Subject: [PATCH 07/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fix open_file().

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/open.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 4071f0f..b7ac221 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -809,6 +809,7 @@ static NTSTATUS open_file(files_struct *fsp,
 			wild = smb_fname->base_name;
 		}
 		if ((local_flags & O_CREAT) && !file_existed &&
+		    !(fsp->posix_flags & FSP_POSIX_FLAGS_PATHNAMES) &&
 		    ms_has_wild(wild))  {
 			return NT_STATUS_OBJECT_NAME_INVALID;
 		}
-- 
2.5.0


From f402628cfe6843d00e1a9fe8df16f8a1839d5f0e Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:51:58 -0800
Subject: [PATCH 08/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fixup reply_search().

Don't think any client makes this call with POSIX extensions
on, but this keeps the same old behavior.

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/reply.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 3fad39b..9234978 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1749,7 +1749,9 @@ void reply_search(struct smb_request *req)
 		 * For a 'continue' search we have no string. So
 		 * check from the initial saved string.
 		 */
-		mask_contains_wcard = ms_has_wild(mask);
+		if (!lp_posix_pathnames()) {
+			mask_contains_wcard = ms_has_wild(mask);
+		}
 		dirtype = dptr_attr(sconn, dptr_num);
 	}
 
-- 
2.5.0


From 3e7b26b1b30e426e239807b16fa86bebce49f89d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:53:30 -0800
Subject: [PATCH 09/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Prepare to remove lp_posix_pathnames() out of ms_has_wild().
Check before calls to ms_has_wild().

Fix smbd_smb2_query_directory_send().

No SMB2 client uses unix extensions yet, but this is a placeholder
for when we move the POSIX pathnames bit into the SMB2 request
when moving to handle based code.

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/smbd/smb2_query_directory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/smb2_query_directory.c b/source3/smbd/smb2_query_directory.c
index 81f2e17..73f2d63 100644
--- a/source3/smbd/smb2_query_directory.c
+++ b/source3/smbd/smb2_query_directory.c
@@ -325,7 +325,9 @@ static struct tevent_req *smbd_smb2_query_directory_send(TALLOC_CTX *mem_ctx,
 		dptr_CloseDir(fsp);
 	}
 
-	wcard_has_wild = ms_has_wild(in_file_name);
+	if (!lp_posix_pathnames()) {
+		wcard_has_wild = ms_has_wild(in_file_name);
+	}
 
 	/* Ensure we've canonicalized any search path if not a wildcard. */
 	if (!wcard_has_wild) {
-- 
2.5.0


From 23391c08f85fdd839147658d7cbc8e2a54fd3115 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 11 Dec 2015 14:55:10 -0800
Subject: [PATCH 10/10] s3: smbd: Moving lp_posix_pathnames() out of the
 lower-level code.

Remove lp_posix_pathnames() out of ms_has_wild().

NB. The usage of ms_has_wild() inside set_namearray()
should *never* have been looking at lp_posix_pathnames()
anyway, as this is a config option that needs to look
at wildcards. This was probably an old (but never
triggered) bug.

Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
 source3/lib/util.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/source3/lib/util.c b/source3/lib/util.c
index 420fe91..1d04318 100644
--- a/source3/lib/util.c
+++ b/source3/lib/util.c
@@ -1611,11 +1611,6 @@ bool ms_has_wild(const char *s)
 {
 	char c;
 
-	if (lp_posix_pathnames()) {
-		/* With posix pathnames no characters are wild. */
-		return False;
-	}
-
 	while ((c = *s++)) {
 		switch (c) {
 		case '*':
-- 
2.5.0



More information about the samba-technical mailing list