[PATCH] Fix failure to update dirpath
Jeremy Allison
jra at samba.org
Wed Aug 22 20:51:00 UTC 2018
On Wed, Aug 22, 2018 at 04:35:16PM +0200, Michael Adam wrote:
>
> I do actually think that this is conceptionally
> the much better patch. It fixes the slightly
> arbitrary (and undocumented) behavior to return
> "" for dirpath in the non-optimized case and had
> all callers react to that arbitrary fact. But the
> dirpath was not really useful for other processing.
>
> With this change you always return a valid
> dirpath, and we don't need to change it
> (to ".") for some callers...
>
> A few more cosmetic review commments below in the patch.
OK, now the "simple" fix has gone in, here is the
cleanup patchset for unix_convert() to ensure we
always have a valid dirpath (incorporates your
comments).
Please review and let me know if it's OK.
Thanks,
Jeremy.
-------------- next part --------------
From b70a1b301f1dfba94b2aac314922ac4c017a2d2b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 22 Aug 2018 09:09:27 -0700
Subject: [PATCH 1/3] s3: smbd: Initialization cleanup.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/filename.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 41c1710351e..2071ddc58d6 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -164,13 +164,12 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
char **pp_dirpath,
char **pp_start)
{
- struct smb_filename parent_fname;
+ struct smb_filename parent_fname = {0};
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,
&parent_fname.base_name,
&last_component)) {
--
2.18.0.1017.ga543ac7ca45-goog
From 1354975735024324f098298344ea558cb6aaed8c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 22 Aug 2018 09:15:12 -0700
Subject: [PATCH 2/3] s3: smbd: Restructure check_parent_exists() to ensure all
non-optimization returns go to one place.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/filename.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 2071ddc58d6..479d532327a 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -168,7 +168,6 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
const char *last_component = NULL;
NTSTATUS status;
int ret;
- bool parent_fname_has_wild = false;
if (!parent_dirname(ctx, smb_fname->base_name,
&parent_fname.base_name,
@@ -177,18 +176,18 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
}
if (!posix_pathnames) {
- parent_fname_has_wild = ms_has_wild(parent_fname.base_name);
+ if (ms_has_wild(parent_fname.base_name)) {
+ goto no_optimization_out;
+ }
}
/*
* If there was no parent component in
- * smb_fname->base_name of the parent name
- * contained a wildcard then don't do this
+ * smb_fname->base_name then don't do this
* optimization.
*/
- if ((smb_fname->base_name == last_component) ||
- parent_fname_has_wild) {
- return NT_STATUS_OK;
+ if (smb_fname->base_name == last_component) {
+ goto no_optimization_out;
}
if (posix_pathnames) {
@@ -201,7 +200,7 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
with the normal tree walk. */
if (ret == -1) {
- return NT_STATUS_OK;
+ goto no_optimization_out;
}
status = check_for_dot_component(&parent_fname);
@@ -234,6 +233,10 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
*pp_start));
return NT_STATUS_OK;
+
+ no_optimization_out:
+
+ return NT_STATUS_OK;
}
/*
--
2.18.0.1017.ga543ac7ca45-goog
From ad6a950340767004e29545e4b44b22a2b9c249d0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 22 Aug 2018 13:37:04 -0700
Subject: [PATCH 3/3] s3: smbd: Ensure dirpath is set to ".", not "\0" so it is
always a valid path.
Cleanup of the internals of unix_convert().
Ensure check_parent_exists() returns this in the non-optimization
case. Ensure unix_convert() initializes dirpath to ".".
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/filename.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 479d532327a..16d0f340102 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -236,6 +236,24 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
no_optimization_out:
+ /*
+ * We must still return an *pp_dirpath
+ * initialized to ".", and a *pp_start
+ * pointing at smb_fname->base_name.
+ */
+
+ TALLOC_FREE(parent_fname.base_name);
+
+ *pp_dirpath = talloc_strdup(ctx, ".");
+ if (*pp_dirpath == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ /*
+ * Safe to use discard_const_p
+ * here as by convention smb_fname->base_name
+ * is allocated off ctx.
+ */
+ *pp_start = discard_const_p(char, smb_fname->base_name);
return NT_STATUS_OK;
}
@@ -601,7 +619,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
goto err;
}
/* dirpath must exist. */
- dirpath = talloc_strdup(ctx,"");
+ dirpath = talloc_strdup(ctx,".");
if (dirpath == NULL) {
status = NT_STATUS_NO_MEMORY;
goto err;
@@ -638,7 +656,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
* building the directories with talloc_asprintf and free it.
*/
- if ((dirpath == NULL) && (!(dirpath = talloc_strdup(ctx,"")))) {
+ if ((dirpath == NULL) && (!(dirpath = talloc_strdup(ctx,".")))) {
DEBUG(0, ("talloc_strdup failed\n"));
status = NT_STATUS_NO_MEMORY;
goto err;
@@ -1038,7 +1056,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
size_t start_ofs =
start - smb_fname->base_name;
- if (*dirpath != '\0') {
+ if (!ISDOT(dirpath)) {
tmp = talloc_asprintf(
smb_fname, "%s/%s",
dirpath, unmangled);
@@ -1073,7 +1091,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
size_t start_ofs =
start - smb_fname->base_name;
- if (*dirpath != '\0') {
+ if (!ISDOT(dirpath)) {
tmp = talloc_asprintf(smb_fname,
"%s/%s/%s", dirpath,
found_name, end+1);
@@ -1098,7 +1116,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
size_t start_ofs =
start - smb_fname->base_name;
- if (*dirpath != '\0') {
+ if (!ISDOT(dirpath)) {
tmp = talloc_asprintf(smb_fname,
"%s/%s", dirpath,
found_name);
@@ -1139,7 +1157,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
* Add to the dirpath that we have resolved so far.
*/
- if (*dirpath != '\0') {
+ if (!ISDOT(dirpath)) {
char *tmp = talloc_asprintf(ctx,
"%s/%s", dirpath, start);
if (!tmp) {
@@ -1209,7 +1227,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
return NT_STATUS_OK;
fail:
DEBUG(10, ("dirpath = [%s] start = [%s]\n", dirpath, start));
- if (dirpath && *dirpath != '\0') {
+ if (dirpath && !ISDOT(dirpath)) {
smb_fname->base_name = talloc_asprintf(smb_fname, "%s/%s",
dirpath, start);
} else {
--
2.18.0.1017.ga543ac7ca45-goog
More information about the samba-technical
mailing list