[PATCHES] sysacls: fix FreeBSD developer build and HPUX POSIX ACL support
Jeremy Allison
jra at samba.org
Thu Dec 21 16:45:49 UTC 2017
On Thu, Dec 21, 2017 at 06:14:19AM +0200, Uri Simchoni wrote:
> 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.
Ah, got it ! Thanks so much for the explaination :-). I went looking
for the mode_t definition in the Linux header files, and when I found
it was uint32_t, assumed that pidl was using the system size.
In that case, RB+ by me - but I think I'll add a comment to
the fix to explain the pidl issue.
Thanks for the explaination.
Jeremy.
> >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