The Samba-Bugzilla – Attachment 18235 Details for
Bug 15549
Symlinks on AIX are broken in 4.19 (and a few version before that)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.19 backported from master
bug15549-v419.patch (text/plain), 17.14 KB, created by
Ralph Böhme
on 2024-01-24 06:46:24 UTC
(
hide
)
Description:
Patch for 4.19 backported from master
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2024-01-24 06:46:24 UTC
Size:
17.14 KB
patch
obsolete
>From 924608bf213a41ad077a5684efde6bec3dd8e432 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 19 Dec 2023 11:11:55 +0100 >Subject: [PATCH 1/6] vfs_default: allow disabling /proc/fds and > RESOLVE_NO_SYMLINK at compile time > >This will be used in CI to have a gitlab runner without all modern Linux >features we make use of as part of path processing: > >- O_PATH >- openat2() with RESOLVE_NO_SYMLINKS >- somehow safely reopen an O_PATH file handle > >That gives what a classix UNIX like AIX or Solaris offers feature wise. > >Other OSes support other combinations of those features, but we leave the >exersize of possibly adding more runners supporting those combinations to the >reader. > >The following list shows which features are available and used by Samba on a few >OSes: > > | O_PATH | RESOLVE_NO_SYMLINKS | Safe reopen | CI covered >--------|----------------|---------------------|---------------------------- > | Supported Used | Supported Used | Supported Used | >============================================================================ >Linux | + + | + + | + + | + >FreeBSD | + + | + [1] - | + [2] - | - >AIX | - - | - - | - - | + > >[1] via open() flag O_RESOLVE_BENEATH >[2] via open() flag O_EMPTY_PATH > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 5c2f96442a25a1725809a28b3719afbc0bd01830) >--- > source3/modules/vfs_default.c | 6 ++++++ > 1 file changed, 6 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index 1d4b9b1a8402..8d78831492f0 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -52,6 +52,9 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const > bool bval; > > handle->conn->have_proc_fds = sys_have_proc_fds(); >+#ifdef DISABLE_PROC_FDS >+ handle->conn->have_proc_fds = false; >+#endif > > /* > * assume the kernel will support openat2(), >@@ -70,6 +73,9 @@ static int vfswrap_connect(vfs_handle_struct *handle, const char *service, const > handle->conn->open_how_resolve |= > VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS; > } >+#ifdef DISABLE_VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS >+ handle->conn->open_how_resolve &= ~VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS; >+#endif > > return 0; /* Return >= 0 for success */ > } >-- >2.43.0 > > >From 926a98f8276763138a556e82da917d8c0ad25a57 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 19 Dec 2023 11:12:49 +0100 >Subject: [PATCH 2/6] CI: disable /proc/fds and RESOLVE_NO_SYMLINK in > samba-no-opath-build runner > >This is a more sensible combination of missing Linux specific features: > >- O_PATH >- openat2() with RESOLVE_NO_SYMLINKS >- somehow safely reopen an O_PATH file handle > >Currently only O_PATH is disabled for these jobs, but that doesn't really match >and know OS. > >The following list shows which features are available and used by Samba on a few >OSes: > > | O_PATH | RESOLVE_NO_SYMLINKS | Safe reopen | CI covered >--------|----------------|---------------------|---------------------------- > | Supported Used | Supported Used | Supported Used | >============================================================================ >Linux | + + | + + | + + | + >FreeBSD | + + | + [1] - | + [2] - | - >AIX | - - | - - | - - | + > >So by also disabling RESOLVE_NO_SYMLINKS and Safe Reopen, we cover classic UNIX >systems like AIX. > >[1] via open() flag O_RESOLVE_BENEATH >[2] via open() flag O_EMPTY_PATH > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 62cbe145c7e500c4759ed2005c78bd5056c87f43) >--- > script/autobuild.py | 2 +- > selftest/skip.opath-required | 6 ++++++ > 2 files changed, 7 insertions(+), 1 deletion(-) > >diff --git a/script/autobuild.py b/script/autobuild.py >index e074c39d3c0b..85043032d73f 100755 >--- a/script/autobuild.py >+++ b/script/autobuild.py >@@ -296,7 +296,7 @@ tasks = { > "samba-no-opath-build": { > "git-clone-required": True, > "sequence": [ >- ("configure", "ADDITIONAL_CFLAGS='-DDISABLE_OPATH=1' ./configure.developer --without-ad-dc " + samba_configure_params), >+ ("configure", "ADDITIONAL_CFLAGS='-DDISABLE_OPATH=1 -DDISABLE_VFS_OPEN_HOW_RESOLVE_NO_SYMLINKS=1 -DDISABLE_PROC_FDS=1' ./configure.developer --without-ad-dc " + samba_configure_params), > ("make", "make -j"), > ("check-clean-tree", CLEAN_SOURCE_TREE_CMD), > ("chmod-R-a-w", "chmod -R a-w ."), >diff --git a/selftest/skip.opath-required b/selftest/skip.opath-required >index c3a13f5ec6e6..67764a0b027c 100644 >--- a/selftest/skip.opath-required >+++ b/selftest/skip.opath-required >@@ -14,3 +14,9 @@ > # available this works fine. So for now restrict testing posix > # extensions to environments where we have O_PATH around > ^samba.tests.smb1posix >+ >+# These don't work without /proc/fd support >+^samba3.blackbox.test_symlink_traversal.*\(fileserver\) >+^samba3.blackbox.shadow_copy_torture.*\(fileserver\) >+^samba3.blackbox.virus_scanner.*\(fileserver:local\) >+^samba3.blackbox.shadow_copy2.*\(fileserver.*\) >-- >2.43.0 > > >From fdc3ce507f8a207a89ce710a5eb0d9c417fb1d1a Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 2 Jan 2024 12:49:14 +0100 >Subject: [PATCH 3/6] smbd: pass symlink target path to > safe_symlink_target_path() > >Moves processing the symlink error response to the caller >filename_convert_dirfsp(). Prepares for using this in >non_widelink_open(), where it will replace symlink_target_below_conn() >with the same functionality. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(back-ported from commit 0515dded4ddb49e5570ae7df51126af1a2d643de) >--- > source3/include/proto.h | 5 +++ > source3/smbd/filename.c | 72 +++++++++++++++++++---------------------- > 2 files changed, 38 insertions(+), 39 deletions(-) > >diff --git a/source3/include/proto.h b/source3/include/proto.h >index 8eed81d8f2e9..13240033bf16 100644 >--- a/source3/include/proto.h >+++ b/source3/include/proto.h >@@ -719,6 +719,11 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx, > const SMB_STRUCT_STAT *psbuf, > NTTIME twrp, > uint32_t flags); >+NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx, >+ const char *connectpath, >+ const char *target, >+ size_t unparsed, >+ char **_relative); > NTSTATUS filename_convert_dirfsp( > TALLOC_CTX *ctx, > connection_struct *conn, >diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c >index 8693dcf1153c..45fb90381e26 100644 >--- a/source3/smbd/filename.c >+++ b/source3/smbd/filename.c >@@ -942,44 +942,34 @@ static char *symlink_target_path( > return ret; > } > >-static NTSTATUS safe_symlink_target_path( >- TALLOC_CTX *mem_ctx, >- const char *connectpath, >- const char *name_in, >- const char *substitute, >- size_t unparsed, >- char **_name_out) >+NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx, >+ const char *connectpath, >+ const char *target, >+ size_t unparsed, >+ char **_relative) > { >- char *target = NULL; > char *abs_target = NULL; > char *abs_target_canon = NULL; > const char *relative = NULL; >- char *name_out = NULL; >- NTSTATUS status = NT_STATUS_NO_MEMORY; > bool in_share; >+ NTSTATUS status = NT_STATUS_NO_MEMORY; > >- target = symlink_target_path(mem_ctx, name_in, substitute, unparsed); >- if (target == NULL) { >- goto fail; >- } >- >- DBG_DEBUG("name_in: %s, substitute: %s, unparsed: %zu, target=%s\n", >- name_in, >- substitute, >- unparsed, >- target); >+ DBG_DEBUG("connectpath [%s] target [%s] unparsed [%zu]\n", >+ connectpath, target, unparsed); > > if (target[0] == '/') { >- abs_target = target; >+ abs_target = talloc_strdup(mem_ctx, target); > } else { >- abs_target = talloc_asprintf( >- target, "%s/%s", connectpath, target); >- if (abs_target == NULL) { >- goto fail; >- } >+ abs_target = talloc_asprintf(mem_ctx, >+ "%s/%s", >+ connectpath, >+ target); >+ } >+ if (abs_target == NULL) { >+ goto fail; > } > >- abs_target_canon = canonicalize_absolute_path(target, abs_target); >+ abs_target_canon = canonicalize_absolute_path(abs_target, abs_target); > if (abs_target_canon == NULL) { > goto fail; > } >@@ -994,15 +984,14 @@ static NTSTATUS safe_symlink_target_path( > goto fail; > } > >- name_out = talloc_strdup(mem_ctx, relative); >- if (name_out == NULL) { >+ *_relative = talloc_strdup(mem_ctx, relative); >+ if (*_relative == NULL) { > goto fail; > } > > status = NT_STATUS_OK; >- *_name_out = name_out; > fail: >- TALLOC_FREE(target); >+ TALLOC_FREE(abs_target); > return status; > } > >@@ -1438,6 +1427,7 @@ NTSTATUS filename_convert_dirfsp( > size_t unparsed = 0; > NTSTATUS status; > char *target = NULL; >+ char *safe_target = NULL; > size_t symlink_redirects = 0; > > next: >@@ -1476,17 +1466,21 @@ NTSTATUS filename_convert_dirfsp( > * resolve all symlinks locally. > */ > >- status = safe_symlink_target_path( >- mem_ctx, >- conn->connectpath, >- name_in, >- substitute, >- unparsed, >- &target); >+ target = symlink_target_path(mem_ctx, name_in, substitute, unparsed); >+ if (target == NULL) { >+ return NT_STATUS_NO_MEMORY; >+ } >+ >+ status = safe_symlink_target_path(mem_ctx, >+ conn->connectpath, >+ target, >+ unparsed, >+ &safe_target); >+ TALLOC_FREE(target); > if (!NT_STATUS_IS_OK(status)) { > return status; > } >- name_in = target; >+ name_in = safe_target; > > symlink_redirects += 1; > >-- >2.43.0 > > >From 2e169a72e9b2458ba0e2861f1330a5b2994a18eb Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 2 Jan 2024 13:25:25 +0100 >Subject: [PATCH 4/6] smbd: add a directory argument to > safe_symlink_target_path() > >Existing caller passes NULL, no change in behaviour. Prepares for >replacing symlink_target_below_conn() in open.c. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit fc80c72d658a41fe4d93b24b793b52c91b350175) >--- > source3/include/proto.h | 1 + > source3/smbd/filename.c | 15 ++++++++++++++- > 2 files changed, 15 insertions(+), 1 deletion(-) > >diff --git a/source3/include/proto.h b/source3/include/proto.h >index 13240033bf16..15c5839caf82 100644 >--- a/source3/include/proto.h >+++ b/source3/include/proto.h >@@ -721,6 +721,7 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx, > uint32_t flags); > NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx, > const char *connectpath, >+ const char *dir, > const char *target, > size_t unparsed, > char **_relative); >diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c >index 45fb90381e26..55a49e0ba93c 100644 >--- a/source3/smbd/filename.c >+++ b/source3/smbd/filename.c >@@ -944,6 +944,7 @@ static char *symlink_target_path( > > NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx, > const char *connectpath, >+ const char *dir, > const char *target, > size_t unparsed, > char **_relative) >@@ -959,10 +960,21 @@ NTSTATUS safe_symlink_target_path(TALLOC_CTX *mem_ctx, > > if (target[0] == '/') { > abs_target = talloc_strdup(mem_ctx, target); >- } else { >+ } else if (dir == NULL) { >+ abs_target = talloc_asprintf(mem_ctx, >+ "%s/%s", >+ connectpath, >+ target); >+ } else if (dir[0] == '/') { > abs_target = talloc_asprintf(mem_ctx, > "%s/%s", >+ dir, >+ target); >+ } else { >+ abs_target = talloc_asprintf(mem_ctx, >+ "%s/%s/%s", > connectpath, >+ dir, > target); > } > if (abs_target == NULL) { >@@ -1473,6 +1485,7 @@ NTSTATUS filename_convert_dirfsp( > > status = safe_symlink_target_path(mem_ctx, > conn->connectpath, >+ NULL, > target, > unparsed, > &safe_target); >-- >2.43.0 > > >From be311f088eac632a1db417ee6cb2d4cd1508820e Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Tue, 2 Jan 2024 14:34:26 +0100 >Subject: [PATCH 5/6] smbd: use safe_symlink_target_path() in > symlink_target_below_conn() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 1965fc77b3852a0593e13897af08f5304a1ce3a2) >--- > selftest/skip.opath-required | 2 - > source3/smbd/open.c | 73 +++++++----------------------------- > 2 files changed, 14 insertions(+), 61 deletions(-) > >diff --git a/selftest/skip.opath-required b/selftest/skip.opath-required >index 67764a0b027c..9c6ba481cdfe 100644 >--- a/selftest/skip.opath-required >+++ b/selftest/skip.opath-required >@@ -16,7 +16,5 @@ > ^samba.tests.smb1posix > > # These don't work without /proc/fd support >-^samba3.blackbox.test_symlink_traversal.*\(fileserver\) > ^samba3.blackbox.shadow_copy_torture.*\(fileserver\) > ^samba3.blackbox.virus_scanner.*\(fileserver:local\) >-^samba3.blackbox.shadow_copy2.*\(fileserver.*\) >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index a7759c349e40..27c96c5de2e4 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -571,7 +571,6 @@ static NTSTATUS chdir_below_conn( > static NTSTATUS symlink_target_below_conn( > TALLOC_CTX *mem_ctx, > const char *connection_path, >- size_t connection_path_len, > struct files_struct *fsp, > struct files_struct *dirfsp, > struct smb_filename *symlink_name, >@@ -579,9 +578,7 @@ static NTSTATUS symlink_target_below_conn( > { > char *target = NULL; > char *absolute = NULL; >- const char *relative = NULL; > NTSTATUS status; >- bool ok; > > if (fsp_get_pathref_fd(fsp) != -1) { > /* >@@ -594,69 +591,28 @@ static NTSTATUS symlink_target_below_conn( > talloc_tos(), dirfsp, symlink_name, &target); > } > >+ status = safe_symlink_target_path(talloc_tos(), >+ connection_path, >+ dirfsp->fsp_name->base_name, >+ target, >+ 0, >+ &absolute); > if (!NT_STATUS_IS_OK(status)) { >- DBG_DEBUG("readlink_talloc failed: %s\n", nt_errstr(status)); >+ DBG_DEBUG("safe_symlink_target_path() failed: %s\n", >+ nt_errstr(status)); > return status; > } > >- if (target[0] != '/') { >- char *tmp = talloc_asprintf( >- talloc_tos(), >- "%s/%s/%s", >- connection_path, >- dirfsp->fsp_name->base_name, >- target); >- >- TALLOC_FREE(target); >- >- if (tmp == NULL) { >- return NT_STATUS_NO_MEMORY; >- } >- target = tmp; >- } >- >- DBG_DEBUG("redirecting to %s\n", target); >- >- absolute = canonicalize_absolute_path(talloc_tos(), target); >- TALLOC_FREE(target); >- >- if (absolute == NULL) { >- return NT_STATUS_NO_MEMORY; >- } >- >- /* >- * We're doing the "below connection_path" here because it's >- * cheap. It might be that we get a symlink out of the share, >- * pointing to yet another symlink getting us back into the >- * share. If we need that, we would have to remove the check >- * here. >- */ >- ok = subdir_of( >- connection_path, >- connection_path_len, >- absolute, >- &relative); >- if (!ok) { >- DBG_NOTICE("Bad access attempt: %s is a symlink " >- "outside the share path\n" >- "conn_rootdir =%s\n" >- "resolved_name=%s\n", >- symlink_name->base_name, >- connection_path, >- absolute); >- TALLOC_FREE(absolute); >- return NT_STATUS_OBJECT_NAME_NOT_FOUND; >- } >- >- if (relative[0] == '\0') { >+ if (absolute[0] == '\0') { > /* > * special case symlink to share root: "." is our > * share root filename > */ >- absolute[0] = '.'; >- absolute[1] = '\0'; >- } else { >- memmove(absolute, relative, strlen(relative)+1); >+ TALLOC_FREE(absolute); >+ absolute = talloc_strdup(talloc_tos(), "."); >+ if (absolute == NULL) { >+ return NT_STATUS_NO_MEMORY; >+ } > } > > *_target = absolute; >@@ -834,7 +790,6 @@ static NTSTATUS non_widelink_open(const struct files_struct *dirfsp, > status = symlink_target_below_conn( > talloc_tos(), > connpath, >- connpath_len, > fsp, > discard_const_p(files_struct, dirfsp), > smb_fname_rel, >-- >2.43.0 > > >From 9ed26172eb71633c9170e02a9cb748d98154a5e0 Mon Sep 17 00:00:00 2001 >From: Ralph Boehme <slow@samba.org> >Date: Mon, 18 Dec 2023 12:35:58 +0100 >Subject: [PATCH 6/6] smbd: use dirfsp and atname in open_directory() > >On systems without /proc/fd support this avoid the expensive chdir() >logic in non_widelink_open(). open_file_ntcreate() already passes >dirfsp and atname to reopen_from_fsp(), it was just missed in the >conversion. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15549 > >Reviewed-by: Volker Lendecke <vl@samba.org> >Signed-off-by: Ralph Boehme <slow@samba.org> > >Autobuild-User(master): Volker Lendecke <vl@samba.org> >Autobuild-Date(master): Mon Jan 22 12:00:56 UTC 2024 on atb-devel-224 > >(cherry picked from commit 2713023250f15cf9971d88620cab9dd4afd0dc73) >--- > source3/smbd/open.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 27c96c5de2e4..c3b279285105 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -4915,8 +4915,8 @@ static NTSTATUS open_directory(connection_struct *conn, > > if (access_mask & need_fd_access) { > status = reopen_from_fsp( >- fsp->conn->cwd_fsp, >- fsp->fsp_name, >+ parent_dir_fname->fsp, >+ smb_fname_atname, > fsp, > O_RDONLY | O_DIRECTORY, > 0, >-- >2.43.0 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18235">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
vl
:
review+
Actions:
View
Attachments on
bug 15549
: 18235 |
18236