[PATCHES] sysacls: fix FreeBSD developer build and HPUX POSIX ACL support
Uri Simchoni
uri at samba.org
Thu Dec 21 04:14:19 UTC 2017
On 12/21/2017 01:19 AM, Jeremy Allison via samba-technical wrote:
> On Wed, Dec 20, 2017 at 10:50:59PM +0200, Uri Simchoni via samba-technical wrote:
>> Hi,
>>
>> The attach patches fix FreeBSD developer build for sysacls module
>> (mode_t is 16 bits and the internal representation of the ACL entry is
>> 32 bits). They also fix a bug of POSIX ACL support on hpux and possibly
>> other big-endian platforms where mode_t is also 16 bits (on Linux mode_t
>> is 32 bits).
>>
>> This is a simpler fix that's been proposed by Volker in response to my
>> previous fix which also simplified the interface, but consisted of a
>> larger patch set.
>>
>> I have confirmation on the existence of the bug in hpux, but not on the
>> fix. However much time has passed without being able to confirm this, so
>> I thought I'd go ahead and submit this to the list.
>>
>> Please review and maybe push :)
>
> OK, I must be being dumb but I don't understand the fix, sorry :-(.
>
> In librpc/idl/smb_acl.idl we have:
>
> typedef struct {
> smb_acl_tag_t a_type;
> [switch_is(a_type)] smb_acl_entry_info info;
> mode_t a_perm;
> } smb_acl_entry;
>
> so a_perm is explicitly defined as a mode_t.
... which PIDL *always* translates to C type of uint32_t, not mode_t.
(see pidl/lib/Parse/Pidl/Typelist.pm)
That's the gist of the bug.
Thanks,
Uri.
>On Linux this
> then gets mapped to a uint32_t, so in the pidl generated file
> bin/default/librpc/gen_ndr/smb_acl.h we have:
>
> struct smb_acl_entry {
> enum smb_acl_tag_t a_type;
> union smb_acl_entry_info info;/* [switch_is(a_type)] */
> uint32_t a_perm;
> };
>
> I'm assuming on FreeBSD this would be:
>
> struct smb_acl_entry {
> enum smb_acl_tag_t a_type;
> union smb_acl_entry_info info;/* [switch_is(a_type)] */
> uint16_t a_perm;
> };
>
> Correct ?
>
> Now in source3/lib/sysacls.c we have:
>
> int sys_acl_get_permset(SMB_ACL_ENTRY_T entry_d, SMB_ACL_PERMSET_T *permset_p)
> {
> *permset_p = &entry_d->a_perm;
>
> return 0;
> }
>
> So SMB_ACL_PERMSET_T should be a pointer to a mode_t, which will get mapped
> to a pointer of the correct size. If you make SMB_ACL_PERMSET_T a pointer
> to a uint32_t, then on FreeBSD isn't it going to be pointing to an element
> of the wrong size ?
>
> Can you explain why this fix works (sorry for being dumb) ?
>
> Jeremy.
>
>> From 61eb0695b22b3bc384066c801995e7d45b0178dc Mon Sep 17 00:00:00 2001
>> From: Uri Simchoni <uri at samba.org>
>> Date: Tue, 5 Dec 2017 20:49:03 +0200
>> Subject: [PATCH 1/2] pysmbd: fix use of sysacl API
>>
>> Fix pysmbd to use the sysacl (POSIX ACL support) as intended, and
>> not assume too much about the inner structure and implementation
>> of the permissions in the sysacl API.
>>
>> This will allow the inner structure to change in a following commit.
>>
>> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13176
>>
>> Signed-off-by: Uri Simchoni <uri at samba.org>
>> ---
>> source3/smbd/pysmbd.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
>> index 63fc5d6..be30b86 100644
>> --- a/source3/smbd/pysmbd.c
>> +++ b/source3/smbd/pysmbd.c
>> @@ -234,6 +234,39 @@ static NTSTATUS get_nt_acl_conn(TALLOC_CTX *mem_ctx,
>> return status;
>> }
>>
>> +static int set_acl_entry_perms(SMB_ACL_ENTRY_T entry, mode_t perm_mask)
>> +{
>> + SMB_ACL_PERMSET_T perms = NULL;
>> +
>> + if (sys_acl_get_permset(entry, &perms) != 0) {
>> + return -1;
>> + }
>> +
>> + if (sys_acl_clear_perms(perms) != 0) {
>> + return -1;
>> + }
>> +
>> + if ((perm_mask & SMB_ACL_READ) != 0 &&
>> + sys_acl_add_perm(perms, SMB_ACL_READ) != 0) {
>> + return -1;
>> + }
>> +
>> + if ((perm_mask & SMB_ACL_WRITE) != 0 &&
>> + sys_acl_add_perm(perms, SMB_ACL_WRITE) != 0) {
>> + return -1;
>> + }
>> +
>> + if ((perm_mask & SMB_ACL_EXECUTE) != 0 &&
>> + sys_acl_add_perm(perms, SMB_ACL_EXECUTE) != 0) {
>> + return -1;
>> + }
>> +
>> + if (sys_acl_set_permset(entry, perms) != 0) {
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>>
>> static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
>> {
>> @@ -261,7 +294,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 (set_acl_entry_perms(entry, mode_user) != 0) {
>> TALLOC_FREE(frame);
>> return NULL;
>> }
>> @@ -276,7 +309,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 (set_acl_entry_perms(entry, mode_group) != 0) {
>> TALLOC_FREE(frame);
>> return NULL;
>> }
>> @@ -291,7 +324,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 (set_acl_entry_perms(entry, mode_other) != 0) {
>> TALLOC_FREE(frame);
>> return NULL;
>> }
>> @@ -312,7 +345,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 (set_acl_entry_perms(entry, mode_group) != 0) {
>> TALLOC_FREE(frame);
>> return NULL;
>> }
>> @@ -328,7 +361,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 (set_acl_entry_perms(entry, mode) != 0) {
>> TALLOC_FREE(frame);
>> return NULL;
>> }
>> --
>> 2.9.5
>>
>>
>> From 5e8d89d6f9a3bf84fef35f3e4584ff6844b00b38 Mon Sep 17 00:00:00 2001
>> From: Uri Simchoni <uri at samba.org>
>> Date: Tue, 5 Dec 2017 20:56:49 +0200
>> Subject: [PATCH 2/2] sysacls: change datatypes to 32 bits
>>
>> The SMB_ACL_PERMSET_T and SMB_ACL_PERM_T were defined as
>> mode_t, which is 16-bits on some (non-Linux) systems. That
>> created a bug on big-endian systems. Changing to 32 bits fixes
>> that.
>>
>> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13176
>>
>> Signed-off-by: Uri Simchoni <uri at samba.org>
>> ---
>> source3/include/smb_acls.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/source3/include/smb_acls.h b/source3/include/smb_acls.h
>> index 73b67af..74fab6c 100644
>> --- a/source3/include/smb_acls.h
>> +++ b/source3/include/smb_acls.h
>> @@ -27,8 +27,8 @@ struct files_struct;
>> struct smb_filename;
>>
>> typedef int SMB_ACL_TYPE_T;
>> -typedef mode_t *SMB_ACL_PERMSET_T;
>> -typedef mode_t SMB_ACL_PERM_T;
>> +typedef uint32_t *SMB_ACL_PERMSET_T;
>> +typedef uint32_t SMB_ACL_PERM_T;
>>
>> typedef enum smb_acl_tag_t SMB_ACL_TAG_T;
>> typedef struct smb_acl_t *SMB_ACL_T;
>> --
>> 2.9.5
>>
>
>
More information about the samba-technical
mailing list