[PATCHES] s3-sysacls - modify SMB_ACL_PERMSET_T to be a mode_t (possible HPUX fix)

Uri Simchoni uri at samba.org
Tue Nov 28 05:39:43 UTC 2017


Hi,

This patch set fixes clang picky-developer errors on FreeBSD around
POSIX acls. It possibly also fixes an hpux issue reported in
https://bugzilla.samba.org/show_bug.cgi?id=11490#c4

I've solicited response on that bug in bugzilla - we need a bug report
and official verification in order to add a BUG: and backport, but even
without bug report, it fixes picky-developer on FreeBSD.

The first patch is the real fix (both to warning and possible bug), and
the rest are cleanup patches (I suppose this can be further cleaned up
by removing return code and error checking from APIs that cannot fail,
and possibly by removing SMB_ACL_PERMSET_T altogether and using mode_t.
There seems to be no end to cleanup...)

Review appreciated,
Uri.
-------------- next part --------------
From 41db3836b5c1d63fc583a175e57e8ad6423f67f4 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 22:24:00 +0200
Subject: [PATCH 1/8] sysacls: make SMB_ACL_PERMSET_T a mode_t

This change turns SMB_ACL_PERMSET_T from a mode_t* into
a mode_t. An SMB_ACL_PERMSET_T is supposed to somehow
represent a set of boolean flags (some combination of
read, write, and execute). A bitmask is sufficient for
that, and there's no need to use pointers.

The real motivation for this patch is to fix a bad pointer
conversion that breaks picky-developer build on FreeBSD,
and might actually introduce a bug in big-endian
systems with mode_t defined as a 16-bit value (probably HPUX).

Before this change, SMB_ACL_PERMSET_T was a pointer. This
was probably so because the POSIX ACL API works that way,
and the original code was probably written before the VFS
era, so the SMB_ACL_PERMSET_T was some alias for the
underlying OS acl_permset_t. Being a pointer,
sys_acl_get_permset() would return  a pointer to a the
permissions mask inside the ACL, which is a uint32_t. On
some OS's (e.g. FreeBSD), a mode_t is a uint16_t, so the code
casts a pointer to a pointer to different size. That triggers
an error in picky developer mode, and, more importantly, breaks
things on big-endian systems.

The change to by-value semantics fixes the issue because the 32-bit
value is properly cast to 16-bit, no matter the endianness.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/include/smb_acls.h       |  6 +++---
 source3/lib/sysacls.c            | 12 ++++++------
 source3/modules/vfs_hpuxacl.c    |  2 +-
 source3/modules/vfs_solarisacl.c |  2 +-
 source3/modules/vfs_tru64acl.c   |  2 +-
 source3/smbd/posix_acls.c        | 16 ++++++++--------
 source3/smbd/pysmbd.c            | 10 +++++-----
 7 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/source3/include/smb_acls.h b/source3/include/smb_acls.h
index 73b67af..e3f7f72 100644
--- a/source3/include/smb_acls.h
+++ b/source3/include/smb_acls.h
@@ -27,7 +27,7 @@ struct files_struct;
 struct smb_filename;
 
 typedef int			SMB_ACL_TYPE_T;
-typedef mode_t			*SMB_ACL_PERMSET_T;
+typedef mode_t			SMB_ACL_PERMSET_T;
 typedef mode_t			SMB_ACL_PERM_T;
 
 typedef enum smb_acl_tag_t SMB_ACL_TAG_T;
@@ -41,8 +41,8 @@ int sys_acl_get_entry(SMB_ACL_T acl_d, int entry_id, SMB_ACL_ENTRY_T *entry_p);
 int sys_acl_get_tag_type(SMB_ACL_ENTRY_T entry_d, SMB_ACL_TAG_T *type_p);
 int sys_acl_get_permset(SMB_ACL_ENTRY_T entry_d, SMB_ACL_PERMSET_T *permset_p);
 void *sys_acl_get_qualifier(SMB_ACL_ENTRY_T entry_d);
-int sys_acl_clear_perms(SMB_ACL_PERMSET_T permset_d);
-int sys_acl_add_perm(SMB_ACL_PERMSET_T permset_d, SMB_ACL_PERM_T perm);
+int sys_acl_clear_perms(SMB_ACL_PERMSET_T *permset_d);
+int sys_acl_add_perm(SMB_ACL_PERMSET_T *permset_d, SMB_ACL_PERM_T perm);
 int sys_acl_get_perm(SMB_ACL_PERMSET_T permset_d, SMB_ACL_PERM_T perm);
 char *sys_acl_to_text(const struct smb_acl_t *acl_d, ssize_t *len_p);
 SMB_ACL_T sys_acl_init(TALLOC_CTX *mem_ctx);
diff --git a/source3/lib/sysacls.c b/source3/lib/sysacls.c
index 0bf3c37..12eccec 100644
--- a/source3/lib/sysacls.c
+++ b/source3/lib/sysacls.c
@@ -95,7 +95,7 @@ int sys_acl_get_tag_type(SMB_ACL_ENTRY_T entry_d, SMB_ACL_TAG_T *type_p)
 
 int sys_acl_get_permset(SMB_ACL_ENTRY_T entry_d, SMB_ACL_PERMSET_T *permset_p)
 {
-	*permset_p = &entry_d->a_perm;
+	*permset_p = entry_d->a_perm;
 
 	return 0;
 }
@@ -114,14 +114,14 @@ void *sys_acl_get_qualifier(SMB_ACL_ENTRY_T entry_d)
 	return NULL;
 }
 
-int sys_acl_clear_perms(SMB_ACL_PERMSET_T permset_d)
+int sys_acl_clear_perms(SMB_ACL_PERMSET_T *permset_d)
 {
 	*permset_d = 0;
 
 	return 0;
 }
 
-int sys_acl_add_perm(SMB_ACL_PERMSET_T permset_d, SMB_ACL_PERM_T perm)
+int sys_acl_add_perm(SMB_ACL_PERMSET_T *permset_d, SMB_ACL_PERM_T perm)
 {
 	if (perm != SMB_ACL_READ && perm != SMB_ACL_WRITE
 	    && perm != SMB_ACL_EXECUTE) {
@@ -141,7 +141,7 @@ int sys_acl_add_perm(SMB_ACL_PERMSET_T permset_d, SMB_ACL_PERM_T perm)
 
 int sys_acl_get_perm(SMB_ACL_PERMSET_T permset_d, SMB_ACL_PERM_T perm)
 {
-	return *permset_d & perm;
+	return permset_d & perm;
 }
 
 char *sys_acl_to_text(const struct smb_acl_t *acl_d, ssize_t *len_p)
@@ -331,12 +331,12 @@ int sys_acl_set_qualifier(SMB_ACL_ENTRY_T entry_d, void *qual_p)
 
 int sys_acl_set_permset(SMB_ACL_ENTRY_T entry_d, SMB_ACL_PERMSET_T permset_d)
 {
-	if (*permset_d & ~(SMB_ACL_READ|SMB_ACL_WRITE|SMB_ACL_EXECUTE)) {
+	if (permset_d & ~(SMB_ACL_READ|SMB_ACL_WRITE|SMB_ACL_EXECUTE)) {
 		errno = EINVAL;
 		return -1;
 	}
 
-	entry_d->a_perm = *permset_d;
+	entry_d->a_perm = permset_d;
 
 	return 0;
 }
diff --git a/source3/modules/vfs_hpuxacl.c b/source3/modules/vfs_hpuxacl.c
index 53e0ad6..76b53cd 100644
--- a/source3/modules/vfs_hpuxacl.c
+++ b/source3/modules/vfs_hpuxacl.c
@@ -529,7 +529,7 @@ static SMB_ACL_T hpux_acl_to_smb_acl(HPUX_ACL_T hpux_acl, int count,
 		/* intentionally not checking return code here: */
 		sys_acl_set_qualifier(smb_entry, (void *)&hpux_acl[i].a_id);
 		smb_perm = hpux_perm_to_smb_perm(hpux_acl[i].a_perm);
-		if (sys_acl_set_permset(smb_entry, &smb_perm) != 0) {
+		if (sys_acl_set_permset(smb_entry, smb_perm) != 0) {
 			DEBUG(10, ("invalid permset given: %d\n", 
 				   hpux_acl[i].a_perm));
 			goto fail;
diff --git a/source3/modules/vfs_solarisacl.c b/source3/modules/vfs_solarisacl.c
index 5c01139..e1750fa 100644
--- a/source3/modules/vfs_solarisacl.c
+++ b/source3/modules/vfs_solarisacl.c
@@ -472,7 +472,7 @@ static SMB_ACL_T solaris_acl_to_smb_acl(SOLARIS_ACL_T solaris_acl, int count,
 		/* intentionally not checking return code here: */
 		sys_acl_set_qualifier(smb_entry, (void *)&solaris_acl[i].a_id);
 		smb_perm = solaris_perm_to_smb_perm(solaris_acl[i].a_perm);
-		if (sys_acl_set_permset(smb_entry, &smb_perm) != 0) {
+		if (sys_acl_set_permset(smb_entry, smb_perm) != 0) {
 			DEBUG(10, ("invalid permset given: %d\n", 
 				   solaris_acl[i].a_perm));
 			goto fail;
diff --git a/source3/modules/vfs_tru64acl.c b/source3/modules/vfs_tru64acl.c
index d44e56a..7d1ceeb 100644
--- a/source3/modules/vfs_tru64acl.c
+++ b/source3/modules/vfs_tru64acl.c
@@ -236,7 +236,7 @@ static bool tru64_ace_to_smb_ace(acl_entry_t tru64_ace,
 		return False;
 	}
 	smb_permset = tru64_permset_to_smb(*permset);
-	if (sys_acl_set_permset(smb_ace, &smb_permset) != 0) {
+	if (sys_acl_set_permset(smb_ace, smb_permset) != 0) {
 		DEBUG(3, ("sys_acl_set_permset failed: %s\n", strerror(errno)));
 		return False;
 	}
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index e4b16b9..4cbb350 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -891,18 +891,18 @@ static mode_t unix_perms_to_acl_perms(mode_t mode, int r_mask, int w_mask, int x
 
 static int map_acl_perms_to_permset(connection_struct *conn, mode_t mode, SMB_ACL_PERMSET_T *p_permset)
 {
-	if (sys_acl_clear_perms(*p_permset) ==  -1)
+	if (sys_acl_clear_perms(p_permset) ==  -1)
 		return -1;
 	if (mode & S_IRUSR) {
-		if (sys_acl_add_perm(*p_permset, SMB_ACL_READ) == -1)
+		if (sys_acl_add_perm(p_permset, SMB_ACL_READ) == -1)
 			return -1;
 	}
 	if (mode & S_IWUSR) {
-		if (sys_acl_add_perm(*p_permset, SMB_ACL_WRITE) == -1)
+		if (sys_acl_add_perm(p_permset, SMB_ACL_WRITE) == -1)
 			return -1;
 	}
 	if (mode & S_IXUSR) {
-		if (sys_acl_add_perm(*p_permset, SMB_ACL_EXECUTE) == -1)
+		if (sys_acl_add_perm(p_permset, SMB_ACL_EXECUTE) == -1)
 			return -1;
 	}
 	return 0;
@@ -4280,22 +4280,22 @@ static bool unix_ex_wire_to_permset(connection_struct *conn, unsigned char wire_
 		return False;
 	}
 
-	if (sys_acl_clear_perms(*p_permset) ==  -1) {
+	if (sys_acl_clear_perms(p_permset) ==  -1) {
 		return False;
 	}
 
 	if (wire_perm & SMB_POSIX_ACL_READ) {
-		if (sys_acl_add_perm(*p_permset, SMB_ACL_READ) == -1) {
+		if (sys_acl_add_perm(p_permset, SMB_ACL_READ) == -1) {
 			return False;
 		}
 	}
 	if (wire_perm & SMB_POSIX_ACL_WRITE) {
-		if (sys_acl_add_perm(*p_permset, SMB_ACL_WRITE) == -1) {
+		if (sys_acl_add_perm(p_permset, SMB_ACL_WRITE) == -1) {
 			return False;
 		}
 	}
 	if (wire_perm & SMB_POSIX_ACL_EXECUTE) {
-		if (sys_acl_add_perm(*p_permset, SMB_ACL_EXECUTE) == -1) {
+		if (sys_acl_add_perm(p_permset, SMB_ACL_EXECUTE) == -1) {
 			return False;
 		}
 	}
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 63fc5d6..f31de8b 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -261,7 +261,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode_user) != 0) {
+	if (sys_acl_set_permset(entry, mode_user) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -276,7 +276,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode_group) != 0) {
+	if (sys_acl_set_permset(entry, mode_group) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -291,7 +291,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode_other) != 0) {
+	if (sys_acl_set_permset(entry, mode_other) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
@@ -312,7 +312,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 			return NULL;
 		}
 
-		if (sys_acl_set_permset(entry, &mode_group) != 0) {
+		if (sys_acl_set_permset(entry, mode_group) != 0) {
 			TALLOC_FREE(frame);
 			return NULL;
 		}
@@ -328,7 +328,7 @@ static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
 		return NULL;
 	}
 
-	if (sys_acl_set_permset(entry, &mode) != 0) {
+	if (sys_acl_set_permset(entry, mode) != 0) {
 		TALLOC_FREE(frame);
 		return NULL;
 	}
-- 
2.9.5


From 44fe086b0ff39cb8d4298622f4fcf0bc42d5e4f3 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 28 Nov 2017 07:16:20 +0200
Subject: [PATCH 2/8] smbd-posixacls: style fixes around previous code changes

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/posix_acls.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 4cbb350..dc8903e 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -889,21 +889,28 @@ static mode_t unix_perms_to_acl_perms(mode_t mode, int r_mask, int w_mask, int x
  an SMB_ACL_PERMSET_T.
 ****************************************************************************/
 
-static int map_acl_perms_to_permset(connection_struct *conn, mode_t mode, SMB_ACL_PERMSET_T *p_permset)
+static int map_acl_perms_to_permset(connection_struct *conn,
+				    mode_t mode,
+				    SMB_ACL_PERMSET_T
+				    *p_permset)
 {
-	if (sys_acl_clear_perms(p_permset) ==  -1)
+	if (sys_acl_clear_perms(p_permset) ==  -1) {
 		return -1;
+	}
 	if (mode & S_IRUSR) {
-		if (sys_acl_add_perm(p_permset, SMB_ACL_READ) == -1)
+		if (sys_acl_add_perm(p_permset, SMB_ACL_READ) == -1) {
 			return -1;
+		}
 	}
 	if (mode & S_IWUSR) {
-		if (sys_acl_add_perm(p_permset, SMB_ACL_WRITE) == -1)
+		if (sys_acl_add_perm(p_permset, SMB_ACL_WRITE) == -1) {
 			return -1;
+		}
 	}
 	if (mode & S_IXUSR) {
-		if (sys_acl_add_perm(p_permset, SMB_ACL_EXECUTE) == -1)
+		if (sys_acl_add_perm(p_permset, SMB_ACL_EXECUTE) == -1) {
 			return -1;
+		}
 	}
 	return 0;
 }
@@ -4274,29 +4281,33 @@ int fchmod_acl(files_struct *fsp, mode_t mode)
  Map from wire type to permset.
 ****************************************************************************/
 
-static bool unix_ex_wire_to_permset(connection_struct *conn, unsigned char wire_perm, SMB_ACL_PERMSET_T *p_permset)
+static bool unix_ex_wire_to_permset(connection_struct *conn,
+				    unsigned char wire_perm,
+				    SMB_ACL_PERMSET_T *p_permset)
 {
-	if (wire_perm & ~(SMB_POSIX_ACL_READ|SMB_POSIX_ACL_WRITE|SMB_POSIX_ACL_EXECUTE)) {
-		return False;
+	if (wire_perm & ~(SMB_POSIX_ACL_READ|
+			  SMB_POSIX_ACL_WRITE|
+			  SMB_POSIX_ACL_EXECUTE)) {
+		return false;
 	}
 
 	if (sys_acl_clear_perms(p_permset) ==  -1) {
-		return False;
+		return false;
 	}
 
 	if (wire_perm & SMB_POSIX_ACL_READ) {
 		if (sys_acl_add_perm(p_permset, SMB_ACL_READ) == -1) {
-			return False;
+			return false;
 		}
 	}
 	if (wire_perm & SMB_POSIX_ACL_WRITE) {
 		if (sys_acl_add_perm(p_permset, SMB_ACL_WRITE) == -1) {
-			return False;
+			return false;
 		}
 	}
 	if (wire_perm & SMB_POSIX_ACL_EXECUTE) {
 		if (sys_acl_add_perm(p_permset, SMB_ACL_EXECUTE) == -1) {
-			return False;
+			return false;
 		}
 	}
 	return True;
-- 
2.9.5


From 9f9df961fb65dc3e9945f69c304e82f6adb260df Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 22:43:51 +0200
Subject: [PATCH 3/8] vfs_aixacl: use SMB_ACL_PERMSET_T instead of
 SMB_ACL_PERM_T

An SMB_ACL_PERM_T represents one permission, and a PERMSET_T
represents a set of permissions. This change doesn't change
compiled code behavior because both are mode_t, but it makes
the code more consistent.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_aixacl_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/vfs_aixacl_util.c b/source3/modules/vfs_aixacl_util.c
index 1194d27..7b8e052 100644
--- a/source3/modules/vfs_aixacl_util.c
+++ b/source3/modules/vfs_aixacl_util.c
@@ -189,7 +189,7 @@ SMB_ACL_T aixacl_to_smbacl(struct acl *file_acl, TALLOC_CTX *mem_ctx)
 
 }
 
-static ushort aixacl_smb_to_aixperm(SMB_ACL_PERM_T a_perm)
+static ushort aixacl_smb_to_aixperm(SMB_ACL_PERMSET_T a_perm)
 {
 	ushort ret = (ushort)0;
 	if (a_perm & SMB_ACL_READ)
-- 
2.9.5


From 162771fc10db2278aef5d8323828cab6907446ba Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 22:46:43 +0200
Subject: [PATCH 4/8] vfs_hpuxacl: use SMB_ACL_PERMSET_T instead of
 SMB_ACL_PERM_T

This change does not modify compiled code behavior as both
types are mode_t, but it makes the code more consistent -
an SMB_ACL_PERMSET_T represents a set of permissions, and
an SMB_ACL_PERM_T represents one permission.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_hpuxacl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_hpuxacl.c b/source3/modules/vfs_hpuxacl.c
index 76b53cd..fec8902 100644
--- a/source3/modules/vfs_hpuxacl.c
+++ b/source3/modules/vfs_hpuxacl.c
@@ -119,8 +119,8 @@ static bool hpux_add_to_acl(HPUX_ACL_T *hpux_acl, int *count,
 		HPUX_ACL_T add_acl, int add_count, SMB_ACL_TYPE_T type);
 static bool hpux_acl_get_file(const char *name, HPUX_ACL_T *hpuxacl, 
 		int *count);
-static SMB_ACL_PERM_T hpux_perm_to_smb_perm(const HPUX_PERM_T perm);
-static HPUX_PERM_T smb_perm_to_hpux_perm(const SMB_ACL_PERM_T perm);
+static SMB_ACL_PERMSET_T hpux_perm_to_smb_perm(HPUX_PERM_T perm);
+static HPUX_PERM_T smb_perm_to_hpux_perm(SMB_ACL_PERMSET_T perm);
 #if 0
 static bool hpux_acl_check(HPUX_ACL_T hpux_acl, int count);
 #endif
@@ -507,8 +507,8 @@ static SMB_ACL_T hpux_acl_to_smb_acl(HPUX_ACL_T hpux_acl, int count,
 		goto fail;
 	}
 	for (i = 0; i < count; i++) {
-		SMB_ACL_ENTRY_T smb_entry;
-		SMB_ACL_PERM_T smb_perm;
+		SMB_ACL_ENTRY_T smb_entry = NULL;
+		SMB_ACL_PERMSET_T smb_perm = 0;
 
 		if (!_IS_OF_TYPE(hpux_acl[i], type)) {
 			continue;
@@ -628,9 +628,9 @@ static SMB_ACL_TAG_T hpux_tag_to_smb_tag(HPUX_ACL_TAG_T hpux_tag)
  * functions are same, but the functions make us independent of the concrete 
  * permission data types.
  */
-static SMB_ACL_PERM_T hpux_perm_to_smb_perm(const HPUX_PERM_T perm)
+static SMB_ACL_PERMSET_T hpux_perm_to_smb_perm(HPUX_PERM_T perm)
 {
-	SMB_ACL_PERM_T smb_perm = 0;
+	SMB_ACL_PERMSET_T smb_perm = 0;
 	smb_perm |= ((perm & SMB_ACL_READ) ? SMB_ACL_READ : 0);
 	smb_perm |= ((perm & SMB_ACL_WRITE) ? SMB_ACL_WRITE : 0);
 	smb_perm |= ((perm & SMB_ACL_EXECUTE) ? SMB_ACL_EXECUTE : 0);
@@ -638,7 +638,7 @@ static SMB_ACL_PERM_T hpux_perm_to_smb_perm(const HPUX_PERM_T perm)
 }
 
 
-static HPUX_PERM_T smb_perm_to_hpux_perm(const SMB_ACL_PERM_T perm)
+static HPUX_PERM_T smb_perm_to_hpux_perm(SMB_ACL_PERMSET_T perm)
 {
 	HPUX_PERM_T hpux_perm = 0;
 	hpux_perm |= ((perm & SMB_ACL_READ) ? SMB_ACL_READ : 0);
-- 
2.9.5


From 9b38ae4dccc61d15c4ea918a3f8e458d4430d746 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 22:49:09 +0200
Subject: [PATCH 5/8] vfs_posixacl: use SMB_ACL_PERMSET_T instead of
 SMB_ACL_PERM_T

This change does not modify compiled code behavior as both
types are mode_t, but it makes the code more consistent -
an SMB_ACL_PERMSET_T represents a set of permissions, and
an SMB_ACL_PERM_T represents one permission.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_posixacl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_posixacl.c b/source3/modules/vfs_posixacl.c
index 0cad9b6..15e7303 100644
--- a/source3/modules/vfs_posixacl.c
+++ b/source3/modules/vfs_posixacl.c
@@ -27,7 +27,7 @@
 static bool smb_ace_to_internal(acl_entry_t posix_ace,
 				struct smb_acl_entry *ace);
 static struct smb_acl_t *smb_acl_to_internal(acl_t acl, TALLOC_CTX *mem_ctx);
-static int smb_acl_set_mode(acl_entry_t entry, SMB_ACL_PERM_T perm);
+static int smb_acl_set_mode(acl_entry_t entry, SMB_ACL_PERMSET_T smb_perm);
 static acl_t smb_acl_to_posix(const struct smb_acl_t *acl);
 
 
@@ -251,7 +251,7 @@ static struct smb_acl_t *smb_acl_to_internal(acl_t acl, TALLOC_CTX *mem_ctx)
 	return result;
 }
 
-static int smb_acl_set_mode(acl_entry_t entry, SMB_ACL_PERM_T perm)
+static int smb_acl_set_mode(acl_entry_t entry, SMB_ACL_PERMSET_T smb_perm)
 {
         int ret;
         acl_permset_t permset;
@@ -262,15 +262,15 @@ static int smb_acl_set_mode(acl_entry_t entry, SMB_ACL_PERM_T perm)
         if ((ret = acl_clear_perms(permset)) != 0) {
                 return ret;
 	}
-        if ((perm & SMB_ACL_READ) &&
+        if ((smb_perm & SMB_ACL_READ) &&
 	    ((ret = acl_add_perm(permset, ACL_READ)) != 0)) {
 		return ret;
 	}
-        if ((perm & SMB_ACL_WRITE) &&
+        if ((smb_perm & SMB_ACL_WRITE) &&
 	    ((ret = acl_add_perm(permset, ACL_WRITE)) != 0)) {
 		return ret;
 	}
-        if ((perm & SMB_ACL_EXECUTE) &&
+        if ((smb_perm & SMB_ACL_EXECUTE) &&
 	    ((ret = acl_add_perm(permset, ACL_EXECUTE)) != 0)) {
 		return ret;
 	}
-- 
2.9.5


From fee028b2f70ca8330fb106d2d77cd2928a327c95 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 22:50:02 +0200
Subject: [PATCH 6/8] vfs_solarisacl: use SMB_ACL_PERMSET_T instead of
 SMB_ACL_PERM_T

This change does not modify compiled code behavior as both
types are mode_t, but it makes the code more consistent -
an SMB_ACL_PERMSET_T represents a set of permissions, and
an SMB_ACL_PERM_T represents one permission.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_solarisacl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_solarisacl.c b/source3/modules/vfs_solarisacl.c
index e1750fa..faf26fc 100644
--- a/source3/modules/vfs_solarisacl.c
+++ b/source3/modules/vfs_solarisacl.c
@@ -54,8 +54,8 @@ static bool solaris_acl_get_file(const char *name, SOLARIS_ACL_T *solarisacl,
 		int *count);
 static bool solaris_acl_get_fd(int fd, SOLARIS_ACL_T *solarisacl, int *count);
 static bool solaris_acl_sort(SOLARIS_ACL_T theacl, int count);
-static SMB_ACL_PERM_T solaris_perm_to_smb_perm(const SOLARIS_PERM_T perm);
-static SOLARIS_PERM_T smb_perm_to_solaris_perm(const SMB_ACL_PERM_T perm);
+static SMB_ACL_PERMSET_T solaris_perm_to_smb_perm(const SOLARIS_PERM_T perm);
+static SOLARIS_PERM_T smb_perm_to_solaris_perm(SMB_ACL_PERMSET_T perm);
 #if 0
 static bool solaris_acl_check(SOLARIS_ACL_T solaris_acl, int count);
 #endif
@@ -450,8 +450,8 @@ static SMB_ACL_T solaris_acl_to_smb_acl(SOLARIS_ACL_T solaris_acl, int count,
 		goto fail;
 	}
 	for (i = 0; i < count; i++) {
-		SMB_ACL_ENTRY_T smb_entry;
-		SMB_ACL_PERM_T smb_perm;
+		SMB_ACL_ENTRY_T smb_entry = NULL;
+		SMB_ACL_PERMSET_T smb_perm = 0;
 		
 		if (!_IS_OF_TYPE(solaris_acl[i], type)) {
 			continue;
@@ -567,9 +567,9 @@ static SMB_ACL_TAG_T solaris_tag_to_smb_tag(SOLARIS_ACL_TAG_T solaris_tag)
 }
 
 
-static SMB_ACL_PERM_T solaris_perm_to_smb_perm(const SOLARIS_PERM_T perm)
+static SMB_ACL_PERMSET_T solaris_perm_to_smb_perm(const SOLARIS_PERM_T perm)
 {
-	SMB_ACL_PERM_T smb_perm = 0;
+	SMB_ACL_PERMSET_T smb_perm = 0;
 	smb_perm |= ((perm & SMB_ACL_READ) ? SMB_ACL_READ : 0);
 	smb_perm |= ((perm & SMB_ACL_WRITE) ? SMB_ACL_WRITE : 0);
 	smb_perm |= ((perm & SMB_ACL_EXECUTE) ? SMB_ACL_EXECUTE : 0);
@@ -577,7 +577,7 @@ static SMB_ACL_PERM_T solaris_perm_to_smb_perm(const SOLARIS_PERM_T perm)
 }
 
 
-static SOLARIS_PERM_T smb_perm_to_solaris_perm(const SMB_ACL_PERM_T perm)
+static SOLARIS_PERM_T smb_perm_to_solaris_perm(const SMB_ACL_PERMSET_T perm)
 {
 	SOLARIS_PERM_T solaris_perm = 0;
 	solaris_perm |= ((perm & SMB_ACL_READ) ? SMB_ACL_READ : 0);
-- 
2.9.5


From f4e1cdc072250980bfc29402db1dd8203c85c1ea Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 22:50:40 +0200
Subject: [PATCH 7/8] vfs_tru64acl: use SMB_ACL_PERMSET_T instead of
 SMB_ACL_PERM_T

This change does not modify compiled code behavior as both
types are mode_t, but it makes the code more consistent -
an SMB_ACL_PERMSET_T represents a set of permissions, and
an SMB_ACL_PERM_T represents one permission.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_tru64acl.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/source3/modules/vfs_tru64acl.c b/source3/modules/vfs_tru64acl.c
index 7d1ceeb..d0fda6d 100644
--- a/source3/modules/vfs_tru64acl.c
+++ b/source3/modules/vfs_tru64acl.c
@@ -31,8 +31,8 @@ static bool tru64_ace_to_smb_ace(acl_entry_t tru64_ace,
 static acl_t smb_acl_to_tru64_acl(const SMB_ACL_T smb_acl);
 static acl_tag_t smb_tag_to_tru64(SMB_ACL_TAG_T smb_tag);
 static SMB_ACL_TAG_T tru64_tag_to_smb(acl_tag_t tru64_tag);
-static acl_perm_t smb_permset_to_tru64(SMB_ACL_PERM_T smb_permset);
-static SMB_ACL_PERM_T tru64_permset_to_smb(const acl_perm_t tru64_permset);
+static acl_perm_t smb_permset_to_tru64(SMB_ACL_PERMSET_T smb_permset);
+static SMB_ACL_PERMSET_T tru64_permset_to_smb(const acl_perm_t tru64_permset);
 
 
 /* public functions - the api */
@@ -203,9 +203,9 @@ static bool tru64_ace_to_smb_ace(acl_entry_t tru64_ace,
 {
 	acl_tag_t tru64_tag;
 	acl_permset_t permset;
-	SMB_ACL_TAG_T smb_tag_type;
-	SMB_ACL_PERM_T smb_permset;
-	void *qualifier;
+	SMB_ACL_TAG_T smb_tag_type = SMB_ACL_TAG_INVALID;
+	SMB_ACL_PERMSET_T smb_permset = 0;
+	void *qualifier = NULL;
 
 	if (acl_get_tag_type(tru64_ace, &tru64_tag) != 0) {
 		DEBUG(0, ("acl_get_tag_type failed: %s\n", strerror(errno)));
@@ -440,7 +440,7 @@ static SMB_ACL_TAG_T tru64_tag_to_smb(acl_tag_t tru64_tag)
 	return smb_tag_type;
 }
 
-static acl_perm_t smb_permset_to_tru64(SMB_ACL_PERM_T smb_permset)
+static acl_perm_t smb_permset_to_tru64(SMB_ACL_PERMSET_T smb_permset)
 {
 	/* originally, I thought that acl_clear_perm was the
 	 * proper way to reset the permset to 0. but without 
@@ -459,9 +459,9 @@ static acl_perm_t smb_permset_to_tru64(SMB_ACL_PERM_T smb_permset)
 	return tru64_permset;
 }
 
-static SMB_ACL_PERM_T tru64_permset_to_smb(const acl_perm_t tru64_permset)
+static SMB_ACL_PERMSET_T tru64_permset_to_smb(const acl_perm_t tru64_permset)
 {
-	SMB_ACL_PERM_T smb_permset  = 0;
+	SMB_ACL_PERMSET_T smb_permset  = 0;
 	smb_permset |= ((tru64_permset & ACL_READ) ? SMB_ACL_READ : 0);
 	smb_permset |= ((tru64_permset & ACL_WRITE) ? SMB_ACL_WRITE : 0);
 	smb_permset |= ((tru64_permset & ACL_EXECUTE) ? SMB_ACL_EXECUTE : 0);
-- 
2.9.5


From 39c21f14c2e2bb782f13422b8c0ed9ebc451fe93 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Mon, 27 Nov 2017 23:01:21 +0200
Subject: [PATCH 8/8] smbd: remove unnecessary uses of sys_acl_get_permset()

After SMB_ACL_PERMSET_T was changed from a pointer to
a value, we no longer have to call sys_acl_get_permset()
just to get a "handle" to the perm-set. sys_acl_get_permset()
is to be called only for fetching the current value.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/posix_acls.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index dc8903e..c3771ac 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -2919,12 +2919,6 @@ static bool set_canon_ace_list(files_struct *fsp,
 		 * Convert the mode_t perms in the canon_ace to a POSIX permset.
 		 */
 
-		if (sys_acl_get_permset(the_entry, &the_permset) == -1) {
-			DEBUG(0,("set_canon_ace_list: Failed to get permset on entry %d. (%s)\n",
-				i, strerror(errno) ));
-			goto fail;
-		}
-
 		if (map_acl_perms_to_permset(conn, p_ace->perms, &the_permset) == -1) {
 			DEBUG(0,("set_canon_ace_list: Failed to create permset for mode (%u) on entry %d. (%s)\n",
 				(unsigned int)p_ace->perms, i, strerror(errno) ));
@@ -2957,11 +2951,6 @@ static bool set_canon_ace_list(files_struct *fsp,
 			goto fail;
 		}
 
-		if (sys_acl_get_permset(mask_entry, &mask_permset) == -1) {
-			DEBUG(0,("set_canon_ace_list: Failed to get mask permset. (%s)\n", strerror(errno) ));
-			goto fail;
-		}
-
 		if (map_acl_perms_to_permset(conn, S_IRUSR|S_IWUSR|S_IXUSR, &mask_permset) == -1) {
 			DEBUG(0,("set_canon_ace_list: Failed to create mask permset. (%s)\n", strerror(errno) ));
 			goto fail;
@@ -4113,9 +4102,6 @@ static int chmod_acl_internals( connection_struct *conn, SMB_ACL_T posix_acl, mo
 		if (sys_acl_get_tag_type(entry, &tagtype) == -1)
 			return -1;
 
-		if (sys_acl_get_permset(entry, &permset) == -1)
-			return -1;
-
 		num_entries++;
 
 		switch(tagtype) {
@@ -4384,13 +4370,6 @@ static SMB_ACL_T create_posix_acl_from_wire(connection_struct *conn,
 			goto fail;
 		}
 
-		/* Get the permset pointer from the new ACL entry. */
-		if (sys_acl_get_permset(the_entry, &the_permset) == -1) {
-			DEBUG(0,("create_posix_acl_from_wire: Failed to get permset on entry %u. (%s)\n",
-                                i, strerror(errno) ));
-                        goto fail;
-                }
-
 		/* Map from wire to permissions. */
 		if (!unix_ex_wire_to_permset(conn, CVAL(pdata,(i*SMB_POSIX_ACL_ENTRY_SIZE)+1), &the_permset)) {
 			DEBUG(0,("create_posix_acl_from_wire: invalid permset %u on entry %u.\n",
-- 
2.9.5



More information about the samba-technical mailing list