From: Volker Lendecke Date: Mon, 28 Sep 2020 13:03:41 +0000 (+0200) Subject: smbclient: Fix recursive mget X-Git-Tag: talloc-2.3.2~424 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9f24b5098f796f364a3f403ad4e9ae28b3c0935a;p=thirdparty%2Fsamba.git smbclient: Fix recursive mget Make do_mget rely on do_list() already doing the recursion in a breadth-first manner. The previous code called do_list() from within its callback. Unfortunately the recent simplifications of do_list() broke this, leading to recursive mget to segfault. Instead of figuring out how this worked before the simplifications in do_list() (I did spend a few hours on this) and fixing it, I chose to restructure do_mget() to not recursively call do_list() anymore but instead rely on do_list() to do the recursion. Saves quite a few lines of code and complexity. Bug: https://bugzilla.samba.org/show_bug.cgi?id=14517 Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Wed Sep 30 17:23:45 UTC 2020 on sn-devel-184 --- diff --git a/selftest/knownfail.d/smbclient_mget b/selftest/knownfail.d/smbclient_mget deleted file mode 100644 index 64407a8c5d4..00000000000 --- a/selftest/knownfail.d/smbclient_mget +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.smbclient-mget.smbclient\ mget\(fileserver\) \ No newline at end of file diff --git a/source3/client/client.c b/source3/client/client.c index d9f4384ef66..d6d5585f305 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -1200,11 +1200,10 @@ static NTSTATUS do_mget(struct cli_state *cli_state, struct file_info *finfo, const char *dir) { TALLOC_CTX *ctx = talloc_tos(); - NTSTATUS status = NT_STATUS_OK; - char *rname = NULL; - char *saved_curdir = NULL; - char *mget_mask = NULL; - char *new_cd = NULL; + const char *client_cwd = NULL; + size_t client_cwd_len; + char *path = NULL; + char *local_path = NULL; if (!finfo->name) { return NT_STATUS_OK; @@ -1213,6 +1212,10 @@ static NTSTATUS do_mget(struct cli_state *cli_state, struct file_info *finfo, if (strequal(finfo->name,".") || strequal(finfo->name,"..")) return NT_STATUS_OK; + if ((finfo->attr & FILE_ATTRIBUTE_DIRECTORY) && !recurse) { + return NT_STATUS_OK; + } + if (prompt) { const char *object = (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) ? "directory" : "file"; @@ -1232,98 +1235,40 @@ static NTSTATUS do_mget(struct cli_state *cli_state, struct file_info *finfo, } } - if (!(finfo->attr & FILE_ATTRIBUTE_DIRECTORY)) { - rname = talloc_asprintf(ctx, - "%s%s", - client_get_cur_dir(), - finfo->name); - if (!rname) { - return NT_STATUS_NO_MEMORY; - } - rname = client_clean_name(ctx, rname); - if (rname == NULL) { - return NT_STATUS_NO_MEMORY; - } - do_get(rname, finfo->name, false); - TALLOC_FREE(rname); - return NT_STATUS_OK; - } - - /* handle directories */ - saved_curdir = talloc_strdup(ctx, client_get_cur_dir()); - if (!saved_curdir) { + path = talloc_asprintf( + ctx, "%s%c%s", dir, CLI_DIRSEP_CHAR, finfo->name); + if (path == NULL) { return NT_STATUS_NO_MEMORY; } - - new_cd = talloc_asprintf(ctx, - "%s%s%s", - client_get_cur_dir(), - finfo->name, - CLI_DIRSEP_STR); - if (!new_cd) { - return NT_STATUS_NO_MEMORY; - } - new_cd = client_clean_name(ctx, new_cd); - if (new_cd == NULL) { + path = client_clean_name(ctx, path); + if (path == NULL) { return NT_STATUS_NO_MEMORY; } - client_set_cur_dir(new_cd); - - string_replace(finfo->name,'\\','/'); - if (lowercase) { - if (!strlower_m(finfo->name)) { - return NT_STATUS_INVALID_PARAMETER; - } - } - - if (!directory_exist(finfo->name) && - mkdir(finfo->name,0777) != 0) { - d_printf("failed to create directory %s\n",finfo->name); - client_set_cur_dir(saved_curdir); - return map_nt_error_from_unix(errno); - } - - if (chdir(finfo->name) != 0) { - d_printf("failed to chdir to directory %s\n",finfo->name); - client_set_cur_dir(saved_curdir); - return map_nt_error_from_unix(errno); - } - mget_mask = talloc_asprintf(ctx, - "%s*", - client_get_cur_dir()); + /* + * Skip the path prefix if we've done a remote "cd" when + * creating the local path + */ + client_cwd = client_get_cur_dir(); + client_cwd_len = strlen(client_cwd); - if (!mget_mask) { + local_path = talloc_strdup(ctx, path + client_cwd_len); + if (local_path == NULL) { + TALLOC_FREE(path); return NT_STATUS_NO_MEMORY; } + string_replace(local_path, CLI_DIRSEP_CHAR, '/'); - mget_mask = client_clean_name(ctx, mget_mask); - if (mget_mask == NULL) { - return NT_STATUS_NO_MEMORY; - } - status = do_list(mget_mask, - (FILE_ATTRIBUTE_SYSTEM - | FILE_ATTRIBUTE_HIDDEN - | FILE_ATTRIBUTE_DIRECTORY), - do_mget, false, true); - if (!NT_STATUS_IS_OK(status) - && !NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { - /* - * Ignore access denied errors to ensure all permitted files are - * pulled down. - */ - return status; - } + if (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) { + int ret = mkdir(local_path, 0777); - if (chdir("..") == -1) { - d_printf("do_mget: failed to chdir to .. (error %s)\n", - strerror(errno) ); - return map_nt_error_from_unix(errno); + if ((ret == -1) && (errno != EEXIST)) { + return map_nt_error_from_unix(errno); + } + } else { + do_get(path, local_path, false); } - client_set_cur_dir(saved_curdir); - TALLOC_FREE(mget_mask); - TALLOC_FREE(saved_curdir); - TALLOC_FREE(new_cd); + return NT_STATUS_OK; } @@ -1430,7 +1375,7 @@ static int cmd_mget(void) if (mget_mask == NULL) { return 1; } - status = do_list(mget_mask, attribute, do_mget, false, true); + status = do_list(mget_mask, attribute, do_mget, recurse, true); if (!NT_STATUS_IS_OK(status)) { return 1; } @@ -1452,7 +1397,7 @@ static int cmd_mget(void) if (mget_mask == NULL) { return 1; } - status = do_list(mget_mask, attribute, do_mget, false, true); + status = do_list(mget_mask, attribute, do_mget, recurse, true); if (!NT_STATUS_IS_OK(status)) { return 1; }