Bug 13176 - POSIX ACL support is broken on hpux and possibly other big-endian OSs
Summary: POSIX ACL support is broken on hpux and possibly other big-endian OSs
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: 4.7.3
Hardware: IA64 HP-UX
: P5 major (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-05 18:47 UTC by Uri Simchoni
Modified: 2018-01-03 08:05 UTC (History)
2 users (show)

See Also:


Attachments
proposed fix for master (3.91 KB, patch)
2017-12-05 19:10 UTC, Uri Simchoni
no flags Details
proposed fix for 4.5.x (3.91 KB, patch)
2017-12-05 19:10 UTC, Uri Simchoni
no flags Details
git-am fix for 4.7.next. (4.59 KB, patch)
2017-12-22 17:52 UTC, Jeremy Allison
uri: review+
Details
git-am fix for 4.6.next (4.60 KB, patch)
2017-12-22 17:52 UTC, Jeremy Allison
uri: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Uri Simchoni 2017-12-05 18:47:43 UTC
This was reported in https://lists.samba.org/archive/samba-technical/2016-November/117232.html , then in https://bugzilla.samba.org/show_bug.cgi?id=11490#c4 (but is not part of that original bug) and re-confirmed recently in https://lists.samba.org/archive/samba-technical/2017-November/124009.html

On big-endian systems, in which mode_t is defined to be a 16-bit value (BSD variants), POSIX ACL is broken, because it returns that all ACL entries have no permissions, preventing file access from any non-admin user.

Some pointer casting causes sys_acl_get_permset() function in SMBD code to return a pointer to a uint32_t field, which is later interpreted as a mode_t pointer. If mode_t is 16 bits and the system is little-endian, the code looks at the wrong bits.
Comment 1 Uri Simchoni 2017-12-05 18:48:21 UTC
On Linux mode_t is 32 bits.
Comment 2 Uri Simchoni 2017-12-05 19:10:19 UTC
Created attachment 13846 [details]
proposed fix for master
Comment 3 Uri Simchoni 2017-12-05 19:10:49 UTC
Created attachment 13847 [details]
proposed fix for 4.5.x
Comment 4 Jeremy Allison 2017-12-20 23:38:04 UTC
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. 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.
Comment 5 Uri Simchoni 2017-12-21 04:24:28 UTC
(In reply to Jeremy Allison from comment #4)
Pidl translates mode_t into uint32_t.

$git grep mode_t "*.pm"
pidl/lib/Parse/Pidl/Typelist.pm:        "mode_t"        => "uint32",
Comment 6 Jeremy Allison 2017-12-22 17:52:04 UTC
Created attachment 13885 [details]
git-am fix for 4.7.next.

Cherry-picked from master.
Comment 7 Jeremy Allison 2017-12-22 17:52:32 UTC
Created attachment 13886 [details]
git-am fix for 4.6.next

Back-ported from master.
Comment 8 Uri Simchoni 2017-12-22 18:01:53 UTC
Assigning to Karolin for inclusion in 4.6.next and 4.7.next
Comment 9 Karolin Seeger 2018-01-02 08:53:33 UTC
(In reply to Uri Simchoni from comment #8)
Pushed to autobuild-v4-{7,6}-test.
Comment 10 Karolin Seeger 2018-01-03 08:05:21 UTC
Pushed to both branches.
Closing out bug report.

Thanks!