Attachment 13157 Details for Bug 12737 – git-am fix for 4.6.next

[patch] git-am fix for 4.6.next

getxattr-4k-46.patch (text/plain), 10.13 KB, created by Uri Simchoni on 2017-04-18 07:32:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Uri Simchoni

Created: 2017-04-18 07:32:33 UTC

Size: 10.13 KB

patch

obsolete

>From 3af3db5ba49e3fd833fd26305ba91875e844e6f7 Mon Sep 17 00:00:00 2001
>From: Uri Simchoni <uri@samba.org>
>Date: Sun, 9 Apr 2017 00:20:40 +0300
>Subject: [PATCH 1/4] selftest: test fetching a large ACL from vfs_acl_xattr
>
>Add a test that fetches an ACL whose size is larger than 4K.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12737
>
>Signed-off-by: Uri Simchoni <uri@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 5017dfeef24b8d568e0146c085f3f979d688acf2)
>---
> source3/script/tests/test_large_acl.sh | 59 ++++++++++++++++++++++++++++++++++
> source3/selftest/tests.py              |  1 +
> 2 files changed, 60 insertions(+)
> create mode 100755 source3/script/tests/test_large_acl.sh
>
>diff --git a/source3/script/tests/test_large_acl.sh b/source3/script/tests/test_large_acl.sh
>new file mode 100755
>index 0000000..9b6901f
>--- /dev/null
>+++ b/source3/script/tests/test_large_acl.sh
>@@ -0,0 +1,59 @@
>+#!/bin/bash
>+#
>+# Blackbox test for fetching a large ACL
>+#
>+
>+if [ $# -lt 5 ]; then
>+cat <<EOF
>+Usage: $0 SERVER USERNAME PASSWORD SMBCLIENT SMBCACLS PARAMS
>+EOF
>+exit 1;
>+fi
>+
>+SERVER=${1}
>+USERNAME=${2}
>+PASSWORD=${3}
>+SMBCLIENT=${4}
>+SMBCACLS=${5}
>+shift 5
>+ADDARGS="$*"
>+SMBCLIENT="$VALGRIND ${SMBCLIENT} ${ADDARGS}"
>+SMBCACLS="$VALGRIND ${SMBCACLS} ${ADDARGS}"
>+
>+incdir=`dirname $0`/../../../testprogs/blackbox
>+. $incdir/subunit.sh
>+
>+# build a file to work with
>+build_files()
>+{
>+    touch large_acl
>+    $SMBCLIENT //$SERVER/acl_xattr_ign_sysacl_windows -U $USERNAME%$PASSWORD -c 'put large_acl' > /dev/null 2>&1
>+    rm -rf large_acl > /dev/null
>+}
>+
>+cleanup()
>+{
>+    $SMBCLIENT //$SERVER/acl_xattr_ign_sysacl_windows -U $USERNAME%$PASSWORD -c 'rm large_acl' > /dev/null 2>&1
>+}
>+
>+build_files
>+
>+test_large_acl()
>+{
>+    #An ACL with 200 entries, ~7K
>+    new_acl=$(seq 1001 1200 | sed -r -e '1 i\D:(A;;0x001f01ff;;;WD)' -e 's/(.*)/(A;;0x001f01ff;;;S-1-5-21-11111111-22222222-33333333-\1)/' | tr -d '\n')
>+    $SMBCACLS //$SERVER/acl_xattr_ign_sysacl_windows -U $USERNAME%$PASSWORD --sddl -S $new_acl large_acl
>+    actual_acl=$($SMBCACLS //$SERVER/acl_xattr_ign_sysacl_windows -U $USERNAME%$PASSWORD --sddl --numeric large_acl 2>/dev/null | sed -rn 's/.*(D:.*)/\1/p' | tr -d '\n')
>+    if [ ! "$new_acl" = "$actual_acl" ] ; then
>+        echo -e "expected:\n$new_acl\nactual:\n$actual_acl\n"
>+        return 1
>+    fi
>+}
>+
>+failed=0
>+
>+testit "able to retrieve a large ACL if VFS supports it" test_large_acl || failed=`expr $failed + 1`
>+
>+cleanup
>+
>+exit $failed
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 4f1a667..8dd2695 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -223,6 +223,7 @@ for env in ["fileserver"]:
>     plantestsuite("samba3.blackbox.inherit_owner.default(%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'tmp', '0', '0', '-m', 'NT1'])
>     plantestsuite("samba3.blackbox.inherit_owner.full (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner', '1', '1', '-m', 'NT1'])
>     plantestsuite("samba3.blackbox.inherit_owner.unix (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner_u', '0', '1', '-m', 'NT1'])
>+    plantestsuite("samba3.blackbox.large_acl (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls])
> 
>     #
>     # tar command tests
>-- 
>2.9.3
>
>
>From a105d1ff470b530024cf0afd87df04c2d46cfdab Mon Sep 17 00:00:00 2001
>From: Uri Simchoni <uri@samba.org>
>Date: Thu, 13 Apr 2017 12:50:47 +0300
>Subject: [PATCH 2/4] vfs_xattr_tdb: handle case of zero size.
>
>With getxattr(), passing a zero buffer size is a
>way of obtaining actual xattr size.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12737
>
>Signed-off-by: Uri Simchoni <uri@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 4dfa2d6a0972847e3b21ddf05077e50ed72c4ea8)
>---
> source3/modules/vfs_xattr_tdb.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/source3/modules/vfs_xattr_tdb.c b/source3/modules/vfs_xattr_tdb.c
>index b32fbc1..58acf44 100644
>--- a/source3/modules/vfs_xattr_tdb.c
>+++ b/source3/modules/vfs_xattr_tdb.c
>@@ -85,6 +85,12 @@ static ssize_t xattr_tdb_getxattr(struct vfs_handle_struct *handle,
> 		TALLOC_FREE(frame);
> 		return -1;
> 	}
>+
>+	if (size == 0) {
>+		TALLOC_FREE(frame);
>+		return xattr_size;
>+	}
>+
> 	if (blob.length > size) {
> 		TALLOC_FREE(frame);
> 		errno = ERANGE;
>@@ -125,6 +131,12 @@ static ssize_t xattr_tdb_fgetxattr(struct vfs_handle_struct *handle,
> 		TALLOC_FREE(frame);
> 		return -1;
> 	}
>+
>+	if (size == 0) {
>+		TALLOC_FREE(frame);
>+		return xattr_size;
>+	}
>+
> 	if (blob.length > size) {
> 		TALLOC_FREE(frame);
> 		errno = ERANGE;
>-- 
>2.9.3
>
>
>From a5f078eea918bedd48d661b51bd0d196f3ef28e4 Mon Sep 17 00:00:00 2001
>From: Uri Simchoni <uri@samba.org>
>Date: Sun, 9 Apr 2017 00:40:44 +0300
>Subject: [PATCH 3/4] vfs_acl_xattr: factor out fetching of an extended
> attribute
>
>Pure refactoring - add a function that fetches an extended attribute
>based on either the file descriptor or the file name.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12737
>
>Signed-off-by: Uri Simchoni <uri@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 7b775abd9278ae34110ec87d94a736be7f64884a)
>---
> source3/modules/vfs_acl_xattr.c | 44 +++++++++++++++++++++++++++++------------
> 1 file changed, 31 insertions(+), 13 deletions(-)
>
>diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
>index e1f90ff..85b9a0d 100644
>--- a/source3/modules/vfs_acl_xattr.c
>+++ b/source3/modules/vfs_acl_xattr.c
>@@ -37,6 +37,35 @@
>  Pull a security descriptor into a DATA_BLOB from a xattr.
> *******************************************************************/
> 
>+static ssize_t getxattr_do(vfs_handle_struct *handle,
>+			   files_struct *fsp,
>+			   const struct smb_filename *smb_fname,
>+			   const char *xattr_name,
>+			   uint8_t *val,
>+			   size_t size)
>+{
>+	ssize_t sizeret;
>+	int saved_errno = 0;
>+
>+	become_root();
>+	if (fsp && fsp->fh->fd != -1) {
>+		sizeret = SMB_VFS_FGETXATTR(fsp, xattr_name, val, size);
>+	} else {
>+		sizeret = SMB_VFS_GETXATTR(handle->conn, smb_fname->base_name,
>+					   XATTR_NTACL_NAME, val, size);
>+	}
>+	if (sizeret == -1) {
>+		saved_errno = errno;
>+	}
>+	unbecome_root();
>+
>+	if (saved_errno != 0) {
>+		errno = saved_errno;
>+	}
>+
>+	return sizeret;
>+}
>+
> static NTSTATUS get_acl_blob(TALLOC_CTX *ctx,
> 			vfs_handle_struct *handle,
> 			files_struct *fsp,
>@@ -47,7 +76,6 @@ static NTSTATUS get_acl_blob(TALLOC_CTX *ctx,
> 	uint8_t *val = NULL;
> 	uint8_t *tmp;
> 	ssize_t sizeret;
>-	int saved_errno = 0;
> 
> 	ZERO_STRUCTP(pblob);
> 
>@@ -60,21 +88,11 @@ static NTSTATUS get_acl_blob(TALLOC_CTX *ctx,
> 	}
> 	val = tmp;
> 
>-	become_root();
>-	if (fsp && fsp->fh->fd != -1) {
>-		sizeret = SMB_VFS_FGETXATTR(fsp, XATTR_NTACL_NAME, val, size);
>-	} else {
>-		sizeret = SMB_VFS_GETXATTR(handle->conn, smb_fname->base_name,
>-					XATTR_NTACL_NAME, val, size);
>-	}
>-	if (sizeret == -1) {
>-		saved_errno = errno;
>-	}
>-	unbecome_root();
>+	sizeret =
>+	    getxattr_do(handle, fsp, smb_fname, XATTR_NTACL_NAME, val, size);
> 
> 	/* Max ACL size is 65536 bytes. */
> 	if (sizeret == -1) {
>-		errno = saved_errno;
> 		if ((errno == ERANGE) && (size != 65536)) {
> 			/* Too small, try again. */
> 			size = 65536;
>-- 
>2.9.3
>
>
>From 4368bf5259c54ecb8ed73044c45d91df829dbe06 Mon Sep 17 00:00:00 2001
>From: Uri Simchoni <uri@samba.org>
>Date: Thu, 13 Apr 2017 12:44:58 +0300
>Subject: [PATCH 4/4] vfs_acl_xattr: avoid needlessly supplying a large buffer
> to getxattr()
>
>When obtaining the security descriptor via getxattr(), first try
>optimistically to supply a buffer of 4K, and if that turns out
>to be too small, determine the correct buffer size.
>
>The previous behavior of falling back to a 64K buffer encountered
>problem with Linux prior to version 3.6, due to pyisical memory
>fragmentation. With those kernels, as long as the buffer is 8K or
>smaller, getting the xattr is much less prone to failure due to
>memory fragmentation.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12737
>
>Signed-off-by: Uri Simchoni <uri@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Tue Apr 18 04:41:16 CEST 2017 on sn-devel-144
>
>(cherry picked from commit 05d83ccf7a6fecf963fcb980acd50cebfc0c3ea9)
>---
> source3/modules/vfs_acl_xattr.c | 44 ++++++++++++++++++++++++++++-------------
> 1 file changed, 30 insertions(+), 14 deletions(-)
>
>diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
>index 85b9a0d..421860b 100644
>--- a/source3/modules/vfs_acl_xattr.c
>+++ b/source3/modules/vfs_acl_xattr.c
>@@ -72,7 +72,7 @@ static NTSTATUS get_acl_blob(TALLOC_CTX *ctx,
> 			const struct smb_filename *smb_fname,
> 			DATA_BLOB *pblob)
> {
>-	size_t size = 1024;
>+	size_t size = 4096;
> 	uint8_t *val = NULL;
> 	uint8_t *tmp;
> 	ssize_t sizeret;
>@@ -91,22 +91,38 @@ static NTSTATUS get_acl_blob(TALLOC_CTX *ctx,
> 	sizeret =
> 	    getxattr_do(handle, fsp, smb_fname, XATTR_NTACL_NAME, val, size);
> 
>-	/* Max ACL size is 65536 bytes. */
>-	if (sizeret == -1) {
>-		if ((errno == ERANGE) && (size != 65536)) {
>-			/* Too small, try again. */
>-			size = 65536;
>-			goto again;
>-		}
>+	if (sizeret >= 0) {
>+		pblob->data = val;
>+		pblob->length = sizeret;
>+		return NT_STATUS_OK;
>+	}
> 
>-		/* Real error - exit here. */
>-		TALLOC_FREE(val);
>-		return map_nt_error_from_unix(errno);
>+	if (errno != ERANGE) {
>+		goto err;
> 	}
> 
>-	pblob->data = val;
>-	pblob->length = sizeret;
>-	return NT_STATUS_OK;
>+	/* Too small, try again. */
>+	sizeret =
>+	    getxattr_do(handle, fsp, smb_fname, XATTR_NTACL_NAME, NULL, 0);
>+	if (sizeret < 0) {
>+		goto err;
>+	}
>+
>+	if (size < sizeret) {
>+		size = sizeret;
>+	}
>+
>+	if (size > 65536) {
>+		/* Max ACL size is 65536 bytes. */
>+		errno = ERANGE;
>+		goto err;
>+	}
>+
>+	goto again;
>+  err:
>+	/* Real error - exit here. */
>+	TALLOC_FREE(val);
>+	return map_nt_error_from_unix(errno);
> }
> 
> /*******************************************************************
>-- 
>2.9.3
>

Flags:

jra: review+

Actions: View

Attachments on bug 12737: 13157 | 13158