The Samba-Bugzilla – Attachment 11018 Details for
Bug 11249
Mangled names do not work with acl_xattr
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Reviewed fix that is going into master.
bug-11249-master (text/plain), 16.37 KB, created by
Jeremy Allison
on 2015-05-04 20:54:11 UTC
(
hide
)
Description:
Reviewed fix that is going into master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2015-05-04 20:54:11 UTC
Size:
16.37 KB
patch
obsolete
>From 263cfa7d4327bba5c2dabdfc898adea69ef73dd8 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 1 May 2015 12:50:51 -0700 >Subject: [PATCH 1/5] s3: smbd: VFS: Add vfs_stat_smb_basename() - to be called > when we *know* stream name parsing has already been done. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11249 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >--- > source3/smbd/proto.h | 2 ++ > source3/smbd/vfs.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+) > >diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h >index a5144d5..5a7d16d 100644 >--- a/source3/smbd/proto.h >+++ b/source3/smbd/proto.h >@@ -1179,6 +1179,8 @@ int vfs_stat_smb_fname(struct connection_struct *conn, const char *fname, > SMB_STRUCT_STAT *psbuf); > int vfs_lstat_smb_fname(struct connection_struct *conn, const char *fname, > SMB_STRUCT_STAT *psbuf); >+int vfs_stat_smb_basename(struct connection_struct *conn, const char *fname, >+ SMB_STRUCT_STAT *psbuf); > NTSTATUS vfs_stat_fsp(files_struct *fsp); > NTSTATUS vfs_chown_fsp(files_struct *fsp, uid_t uid, gid_t gid); > NTSTATUS vfs_streaminfo(connection_struct *conn, >diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c >index ebd3440..997fe17 100644 >--- a/source3/smbd/vfs.c >+++ b/source3/smbd/vfs.c >@@ -1331,6 +1331,32 @@ int vfs_lstat_smb_fname(struct connection_struct *conn, const char *fname, > } > > /** >+ * XXX: This is temporary and there should be no callers of this once >+ * smb_filename is plumbed through all path based operations. >+ * >+ * Called when we know stream name parsing has already been done. >+ */ >+int vfs_stat_smb_basename(struct connection_struct *conn, const char *fname, >+ SMB_STRUCT_STAT *psbuf) >+{ >+ struct smb_filename smb_fname = { >+ .base_name = fname >+ }; >+ int ret; >+ >+ if (lp_posix_pathnames()) { >+ ret = SMB_VFS_LSTAT(conn, &smb_fname); >+ } else { >+ ret = SMB_VFS_STAT(conn, &smb_fname); >+ } >+ >+ if (ret != -1) { >+ *psbuf = smb_fname.st; >+ } >+ return ret; >+} >+ >+/** > * Ensure LSTAT is called for POSIX paths. > */ > >-- >2.2.0.rc0.207.ga3a616c > > >From 92846177318960a6727f33a8a6023abf70429975 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 1 May 2015 13:09:36 -0700 >Subject: [PATCH 2/5] s3: smbd: VFS: All the places that are currently calling > vfs_stat_smb_fname() and vfs_lstat_smb_fname() should be calling > vfs_stat_smb_basename(). > >They are all post-stream name processing. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11249 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >--- > source3/modules/nfs4_acls.c | 4 ++-- > source3/modules/vfs_acl_common.c | 19 ++++++++++++++++++- > source3/modules/vfs_acl_tdb.c | 16 +++------------- > source3/modules/vfs_recycle.c | 2 +- > source3/modules/vfs_solarisacl.c | 2 +- > source3/modules/vfs_xattr_tdb.c | 2 +- > 6 files changed, 26 insertions(+), 19 deletions(-) > >diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c >index adc9b37..f7daa8d 100644 >--- a/source3/modules/nfs4_acls.c >+++ b/source3/modules/nfs4_acls.c >@@ -316,9 +316,9 @@ static int smbacl4_GetFileOwner(struct connection_struct *conn, > memset(psbuf, 0, sizeof(SMB_STRUCT_STAT)); > > /* Get the stat struct for the owner info. */ >- if (vfs_stat_smb_fname(conn, filename, psbuf) != 0) >+ if (vfs_stat_smb_basename(conn, filename, psbuf) != 0) > { >- DEBUG(8, ("vfs_stat_smb_fname failed with error %s\n", >+ DEBUG(8, ("vfs_stat_smb_basename failed with error %s\n", > strerror(errno))); > return -1; > } >diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c >index 920c811..625e7cb 100644 >--- a/source3/modules/vfs_acl_common.c >+++ b/source3/modules/vfs_acl_common.c >@@ -620,7 +620,24 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, > } > psbuf = &fsp->fsp_name->st; > } else { >- int ret = vfs_stat_smb_fname(handle->conn, >+ /* >+ * https://bugzilla.samba.org/show_bug.cgi?id=11249 >+ * >+ * We are currently guaranteed that 'name' here is >+ * a smb_fname->base_name, which *cannot* contain >+ * a stream name (':'). vfs_stat_smb_fname() splits >+ * a name into a base name + stream name, which >+ * when we get here we know we've already done. >+ * So we have to call the stat or lstat VFS >+ * calls directly here. Else, a base_name that >+ * contains a ':' (from a demangled name) will >+ * get split again. >+ * >+ * FIXME. >+ * This uglyness will go away once smb_fname >+ * is fully plumbed through the VFS. >+ */ >+ int ret = vfs_stat_smb_basename(handle->conn, > name, > &sbuf); > if (ret == -1) { >diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c >index 8ee4bd5..e02993b 100644 >--- a/source3/modules/vfs_acl_tdb.c >+++ b/source3/modules/vfs_acl_tdb.c >@@ -159,7 +159,7 @@ static NTSTATUS get_acl_blob(TALLOC_CTX *ctx, > status = vfs_stat_fsp(fsp); > sbuf = fsp->fsp_name->st; > } else { >- int ret = vfs_stat_smb_fname(handle->conn, name, &sbuf); >+ int ret = vfs_stat_smb_basename(handle->conn, name, &sbuf); > if (ret == -1) { > status = map_nt_error_from_unix(errno); > } >@@ -282,12 +282,7 @@ static int rmdir_acl_tdb(vfs_handle_struct *handle, const char *path) > struct db_context *db = acl_db; > int ret = -1; > >- if (lp_posix_pathnames()) { >- ret = vfs_lstat_smb_fname(handle->conn, path, &sbuf); >- } else { >- ret = vfs_stat_smb_fname(handle->conn, path, &sbuf); >- } >- >+ ret = vfs_stat_smb_basename(handle->conn, path, &sbuf); > if (ret == -1) { > return -1; > } >@@ -347,12 +342,7 @@ static int sys_acl_set_file_tdb(vfs_handle_struct *handle, > struct db_context *db = acl_db; > int ret = -1; > >- if (lp_posix_pathnames()) { >- ret = vfs_lstat_smb_fname(handle->conn, path, &sbuf); >- } else { >- ret = vfs_stat_smb_fname(handle->conn, path, &sbuf); >- } >- >+ ret = vfs_stat_smb_basename(handle->conn, path, &sbuf); > if (ret == -1) { > return -1; > } >diff --git a/source3/modules/vfs_recycle.c b/source3/modules/vfs_recycle.c >index 00d7f34..9af78fd 100644 >--- a/source3/modules/vfs_recycle.c >+++ b/source3/modules/vfs_recycle.c >@@ -188,7 +188,7 @@ static bool recycle_directory_exist(vfs_handle_struct *handle, const char *dname > { > SMB_STRUCT_STAT st; > >- if (vfs_stat_smb_fname(handle->conn, dname, &st) == 0) { >+ if (vfs_stat_smb_basename(handle->conn, dname, &st) == 0) { > if (S_ISDIR(st.st_ex_mode)) { > return True; > } >diff --git a/source3/modules/vfs_solarisacl.c b/source3/modules/vfs_solarisacl.c >index 9b3c4f6..efd2d75 100644 >--- a/source3/modules/vfs_solarisacl.c >+++ b/source3/modules/vfs_solarisacl.c >@@ -167,7 +167,7 @@ int solarisacl_sys_acl_set_file(vfs_handle_struct *handle, > * that has not been specified in "type" from the file first > * and concatenate it with the acl provided. > */ >- if (vfs_stat_smb_fname(handle->conn, name, &s) != 0) { >+ if (vfs_stat_smb_basename(handle->conn, name, &s) != 0) { > DEBUG(10, ("Error in stat call: %s\n", strerror(errno))); > goto done; > } >diff --git a/source3/modules/vfs_xattr_tdb.c b/source3/modules/vfs_xattr_tdb.c >index 63a12fd..66c19f8 100644 >--- a/source3/modules/vfs_xattr_tdb.c >+++ b/source3/modules/vfs_xattr_tdb.c >@@ -414,7 +414,7 @@ static int xattr_tdb_rmdir(vfs_handle_struct *handle, const char *path) > TALLOC_FREE(frame); return -1; > }); > >- if (vfs_stat_smb_fname(handle->conn, path, &sbuf) == -1) { >+ if (vfs_stat_smb_basename(handle->conn, path, &sbuf) == -1) { > TALLOC_FREE(frame); > return -1; > } >-- >2.2.0.rc0.207.ga3a616c > > >From db9c52916a587df0c917dc13f65944dfbfddb861 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 1 May 2015 13:13:00 -0700 >Subject: [PATCH 3/5] s3: smbd: VFS: Remove vfs_stat_smb_fname() and > vfs_lstat_smb_fname(). > >No longer used or needed. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11249 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >--- > source3/smbd/proto.h | 4 ---- > source3/smbd/vfs.c | 55 ---------------------------------------------------- > 2 files changed, 59 deletions(-) > >diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h >index 5a7d16d..09a7371 100644 >--- a/source3/smbd/proto.h >+++ b/source3/smbd/proto.h >@@ -1175,10 +1175,6 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname); > NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, > const char *fname, > struct smb_request *smbreq); >-int vfs_stat_smb_fname(struct connection_struct *conn, const char *fname, >- SMB_STRUCT_STAT *psbuf); >-int vfs_lstat_smb_fname(struct connection_struct *conn, const char *fname, >- SMB_STRUCT_STAT *psbuf); > int vfs_stat_smb_basename(struct connection_struct *conn, const char *fname, > SMB_STRUCT_STAT *psbuf); > NTSTATUS vfs_stat_fsp(files_struct *fsp); >diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c >index 997fe17..b83dfcf 100644 >--- a/source3/smbd/vfs.c >+++ b/source3/smbd/vfs.c >@@ -1278,61 +1278,6 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) > /** > * XXX: This is temporary and there should be no callers of this once > * smb_filename is plumbed through all path based operations. >- */ >-int vfs_stat_smb_fname(struct connection_struct *conn, const char *fname, >- SMB_STRUCT_STAT *psbuf) >-{ >- struct smb_filename *smb_fname; >- int ret; >- >- smb_fname = synthetic_smb_fname_split(talloc_tos(), fname, NULL); >- if (smb_fname == NULL) { >- errno = ENOMEM; >- return -1; >- } >- >- if (lp_posix_pathnames()) { >- ret = SMB_VFS_LSTAT(conn, smb_fname); >- } else { >- ret = SMB_VFS_STAT(conn, smb_fname); >- } >- >- if (ret != -1) { >- *psbuf = smb_fname->st; >- } >- >- TALLOC_FREE(smb_fname); >- return ret; >-} >- >-/** >- * XXX: This is temporary and there should be no callers of this once >- * smb_filename is plumbed through all path based operations. >- */ >-int vfs_lstat_smb_fname(struct connection_struct *conn, const char *fname, >- SMB_STRUCT_STAT *psbuf) >-{ >- struct smb_filename *smb_fname; >- int ret; >- >- smb_fname = synthetic_smb_fname_split(talloc_tos(), fname, NULL); >- if (smb_fname == NULL) { >- errno = ENOMEM; >- return -1; >- } >- >- ret = SMB_VFS_LSTAT(conn, smb_fname); >- if (ret != -1) { >- *psbuf = smb_fname->st; >- } >- >- TALLOC_FREE(smb_fname); >- return ret; >-} >- >-/** >- * XXX: This is temporary and there should be no callers of this once >- * smb_filename is plumbed through all path based operations. > * > * Called when we know stream name parsing has already been done. > */ >-- >2.2.0.rc0.207.ga3a616c > > >From ab05f38aefae2f673d99e55907e0dea379a97661 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 1 May 2015 21:06:20 -0700 >Subject: [PATCH 4/5] s3: smbd: VFS: For all EA and ACL calls use > synthetic_smb_fname(), not synthetic_smb_fname_split(). > >EA's and ACL paths are all post-stream name checks (and shouldn't >get stream names). This one took a *long* time to find. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11249 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >--- > source3/modules/non_posix_acls.c | 2 +- > source3/modules/vfs_fake_acls.c | 2 +- > source3/modules/vfs_xattr_tdb.c | 2 +- > source3/smbd/posix_acls.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > >diff --git a/source3/modules/non_posix_acls.c b/source3/modules/non_posix_acls.c >index b1c2420..fca9979 100644 >--- a/source3/modules/non_posix_acls.c >+++ b/source3/modules/non_posix_acls.c >@@ -32,7 +32,7 @@ int non_posix_sys_acl_blob_get_file_helper(vfs_handle_struct *handle, > struct xattr_sys_acl_hash_wrapper acl_wrapper = {}; > struct smb_filename *smb_fname; > >- smb_fname = synthetic_smb_fname_split(frame, path_p, NULL); >+ smb_fname = synthetic_smb_fname(frame, path_p, NULL, NULL); > if (smb_fname == NULL) { > TALLOC_FREE(frame); > errno = ENOMEM; >diff --git a/source3/modules/vfs_fake_acls.c b/source3/modules/vfs_fake_acls.c >index 0e7ebb9..f3c2ebb 100644 >--- a/source3/modules/vfs_fake_acls.c >+++ b/source3/modules/vfs_fake_acls.c >@@ -348,7 +348,7 @@ static int fake_acls_sys_acl_delete_def_file(vfs_handle_struct *handle, const ch > TALLOC_CTX *frame = talloc_stackframe(); > struct smb_filename *smb_fname; > >- smb_fname = synthetic_smb_fname_split(frame, path, NULL); >+ smb_fname = synthetic_smb_fname(frame, path, NULL, NULL); > if (smb_fname == NULL) { > TALLOC_FREE(frame); > errno = ENOMEM; >diff --git a/source3/modules/vfs_xattr_tdb.c b/source3/modules/vfs_xattr_tdb.c >index 66c19f8..2124d38 100644 >--- a/source3/modules/vfs_xattr_tdb.c >+++ b/source3/modules/vfs_xattr_tdb.c >@@ -37,7 +37,7 @@ static int xattr_tdb_get_file_id(struct vfs_handle_struct *handle, > TALLOC_CTX *frame = talloc_stackframe(); > struct smb_filename *smb_fname; > >- smb_fname = synthetic_smb_fname_split(frame, path, NULL); >+ smb_fname = synthetic_smb_fname(frame, path, NULL, NULL); > if (smb_fname == NULL) { > TALLOC_FREE(frame); > errno = ENOMEM; >diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c >index 6a5ec85..8b4c7b1 100644 >--- a/source3/smbd/posix_acls.c >+++ b/source3/smbd/posix_acls.c >@@ -4749,7 +4749,7 @@ int posix_sys_acl_blob_get_file(vfs_handle_struct *handle, > }; > struct smb_filename *smb_fname; > >- smb_fname = synthetic_smb_fname_split(frame, path_p, NULL); >+ smb_fname = synthetic_smb_fname(frame, path_p, NULL, NULL); > if (smb_fname == NULL) { > TALLOC_FREE(frame); > errno = ENOMEM; >-- >2.2.0.rc0.207.ga3a616c > > >From 3a12091852a6b8b6c73761da3389d18819043f97 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 1 May 2015 21:08:21 -0700 >Subject: [PATCH 5/5] s3: torture: Add regression test for bug #11249. > >Bug 11249 - Mangled names do not work with acl_xattr > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11249 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >--- > selftest/target/Samba3.pm | 13 +++++++++++ > source3/script/tests/test_smbclient_s3.sh | 38 +++++++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > >diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm >index 758ca6b..bc52263 100755 >--- a/selftest/target/Samba3.pm >+++ b/selftest/target/Samba3.pm >@@ -980,6 +980,9 @@ sub provision($$$$$$$$) > my $lease2_shrdir="$shrdir/SMB3_00"; > push(@dirs,$lease2_shrdir); > >+ my $manglenames_shrdir="$shrdir/manglenames"; >+ push(@dirs,$manglenames_shrdir); >+ > # this gets autocreated by winbindd > my $wbsockdir="$prefix_abs/winbindd"; > my $wbsockprivdir="$lockdir/winbindd_privileged"; >@@ -1063,6 +1066,12 @@ sub provision($$$$$$$$) > close(BADNAME_TARGET); > chmod 0666, $badname_target; > >+ ## >+ ## create mangleable directory names in $manglenames_shrdir >+ ## >+ my $manglename_target = "$manglenames_shrdir/foo:bar"; >+ mkdir($manglename_target, 0777); >+ > my $conffile="$libdir/server.conf"; > > my $nss_wrapper_pl = "$ENV{PERL} $self->{srcdir}/lib/nss_wrapper/nss_wrapper.pl"; >@@ -1338,6 +1347,10 @@ sub provision($$$$$$$$) > path = $badnames_shrdir > guest ok = yes > >+[manglenames_share] >+ path = $manglenames_shrdir >+ guest ok = yes >+ > [dynamic_share] > path = $shrdir/%R > guest ok = yes >diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh >index e786c9f..faf8d98 100755 >--- a/source3/script/tests/test_smbclient_s3.sh >+++ b/source3/script/tests/test_smbclient_s3.sh >@@ -863,6 +863,40 @@ test_bad_names() > fi > } > >+# Test accessing an share with a name that must be mangled - with acl_xattrs. >+# We know foo:bar gets mangled to FF4GBY~Q with the default name-mangling algorithm (hash2). >+test_mangled_names() >+{ >+ tmpfile=$PREFIX/smbclient_interactive_prompt_commands >+ cat > $tmpfile <<EOF >+ls >+cd FF4GBY~Q >+ls >+quit >+EOF >+ cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/manglenames_share -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' >+ eval echo "$cmd" >+ out=`eval $cmd` >+ ret=$? >+ rm -f $tmpfile >+ >+ if [ $ret != 0 ] ; then >+ echo "$out" >+ echo "failed accessing manglenames_share with error $ret" >+ false >+ return >+ fi >+ >+ echo "$out" | grep 'NT_STATUS' >+ ret=$? >+ if [ $ret == 0 ] ; then >+ echo "$out" >+ echo "failed - NT_STATUS_XXXX listing \\manglenames_share\\FF4GBY~Q" >+ false >+ fi >+} >+ >+ > LOGDIR_PREFIX=test_smbclient_s3 > > # possibly remove old logdirs: >@@ -942,6 +976,10 @@ testit "list a share with bad names (won't convert)" \ > test_bad_names || \ > failed=`expr $failed + 1` > >+testit "list a share with a mangled name + acl_xattr object" \ >+ test_mangled_names || \ >+ failed=`expr $failed + 1` >+ > testit "rm -rf $LOGDIR" \ > rm -rf $LOGDIR || \ > failed=`expr $failed + 1` >-- >2.2.0.rc0.207.ga3a616c >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=11018">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 11249
:
11010
|
11011
|
11018
|
11019
|
11021
|
11022