Bug 9406 - ACL fixes since 4.0 rc1
Summary: ACL fixes since 4.0 rc1
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.0.0rc5
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 9384 8622 9160 9342 9381 9383
  Show dependency treegraph
 
Reported: 2012-11-16 09:08 UTC by Andrew Bartlett
Modified: 2012-12-03 19:32 UTC (History)
4 users (show)

See Also:


Attachments
ACL fixes for 4.0 (263.81 KB, patch)
2012-11-16 09:08 UTC, Andrew Bartlett
abartlet: review+
ambi: review-
Details
ACL fixes for 4.0 including fixes from ambi (270.39 KB, patch)
2012-11-17 05:14 UTC, Andrew Bartlett
no flags Details
ACL fixes for 4.0 including fixes from ambi, and including the AIX build fix (277.89 KB, patch)
2012-11-20 12:22 UTC, Andrew Bartlett
metze: review-
Details
Same patches for v4-0-test incl. cherry-pick info (without unrelated "Rework ldap attribute fetch" patch) See https://bugzilla.samba.org/attachment.cgi?id=8214 (277.43 KB, patch)
2012-12-03 08:19 UTC, Stefan Metzmacher
metze: review+
Details
Additional patches for v4-0-test (part1) (18.17 KB, patch)
2012-12-03 08:21 UTC, Stefan Metzmacher
obnox: review+
Details
Additional patches for v4-0-test (part2) (46.76 KB, patch)
2012-12-03 10:10 UTC, Stefan Metzmacher
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2012-11-16 09:08:10 UTC
Created attachment 8208 [details]
ACL fixes for 4.0

This is the series of ACL fixes since 4.0 rc1

This does not include the new posix acl hash handling (unfinished in master).

The changes here are not in the preferred 'cherry-pick' form, as I can't manage that for this large volume of patches (metze may be able to replace the series).

Because I can't get it to apply cleanly, it also does not include Jeremy's commit cf1540b73714fac6b25de5942cbd821e5f4f6ffc
Author: Jeremy Allison <jra@samba.org>
Date:   Tue Nov 13 11:22:15 2012 -0800

    Another fix needed for bug #9236 - ACL masks incorrectly applied when setting ACLs.

From here, I'll run tests, both manual and automatic, and hopefully we these can get into RC6.
Comment 1 Andrew Bartlett 2012-11-16 09:55:42 UTC
Comment on attachment 8208 [details]
ACL fixes for 4.0

Christian,

Are you aware (eg in GPFS) of any important ACL changes, particularly around the mem_ctx addition, that are not in this patch set?
Comment 2 Andrew Bartlett 2012-11-16 09:56:54 UTC
Metze,

You may wish to replace this patch set with one based on cherry-picked patches, if practical.
Comment 3 Stefan Metzmacher 2012-11-16 10:00:11 UTC
Comment on attachment 8208 [details]
ACL fixes for 4.0

I'll have a closer look,
once I'm finished (I may upload an extended patchset
which cherry-pick information) I'll ask Jeremy for a second review.
Comment 4 Christian Ambach 2012-11-16 21:53:32 UTC
Comment on attachment 8208 [details]
ACL fixes for 4.0

you'll need commit 7a6182962966e5edb42728c8d0a4d471a69c83d7 that fixes the build break that [PATCH 05/42] smbd: Add mem_ctx to sys_acl_init() and all callers introduces.
Unfortunately, that commits adds a memory corruption bug for which I have a patch ready, but not pushed to master yet (+some memory hierarchy fixes in the nfs4acls code which will create additional trouble when using acl_xattr with its use of stackframes)
Comment 5 Andrew Bartlett 2012-11-16 22:18:59 UTC
Christian,

I would like to make progress on this, starting by getting your memory handling changes into master.  What is blocking that, and how can I help?
Comment 6 Andrew Bartlett 2012-11-17 05:14:32 UTC
Created attachment 8211 [details]
ACL fixes for 4.0 including fixes from ambi

New patch including the patches ambi and I just got into master (included just after the mem_ctx patches).
Comment 7 Christian Ambach 2012-11-19 15:28:28 UTC
Comment on attachment 8211 [details]
ACL fixes for 4.0 including fixes from ambi

Are you sure that patch "[PATCH 48/48] samba-tool dns: Don't use "localhost" to connect to local host " is correct for this bug?
Comment 8 Christian Ambach 2012-11-19 15:53:43 UTC
Comment on attachment 8211 [details]
ACL fixes for 4.0 including fixes from ambi

You'll also need to squash this here into [PATCH 06/48] smbd: Add mem_ctx to {f,}get_nt_acl VFS call to make the patch not break the build of vfs_gpfs. master has a different fix for this (fa728d1c) that does not apply here because it also touches the new blob functions.
Comment 9 Christian Ambach 2012-11-19 15:54:02 UTC
diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index 5de984b..c1dc52a 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -363,7 +363,8 @@ static NTSTATUS gpfsacl_fget_nt_acl(vfs_handle_struct *handle,
                                return NT_STATUS_INTERNAL_ERROR);
 
        if (!config->acl) {
-               return SMB_VFS_NEXT_FGET_NT_ACL(handle, fsp, security_info, ppdesc);
+               return SMB_VFS_NEXT_FGET_NT_ACL(handle, fsp, security_info,
+                                               mem_ctx, ppdesc);
        }
 
        result = gpfs_get_nfs4_acl(fsp->fsp_name->base_name, &pacl);
@@ -396,7 +397,8 @@ static NTSTATUS gpfsacl_get_nt_acl(vfs_handle_struct *handle,
                                return NT_STATUS_INTERNAL_ERROR);
 
        if (!config->acl) {
-               return SMB_VFS_NEXT_GET_NT_ACL(handle, name, security_info, ppdesc);
+               return SMB_VFS_NEXT_GET_NT_ACL(handle, name, security_info,
+                                              mem_ctx, ppdesc);
        }
 
        result = gpfs_get_nfs4_acl(name, &pacl);
Comment 10 Christian Ambach 2012-11-19 17:11:20 UTC
I tried to compile v4-0-test + the patchset on AIX and it failed because a0588fd abartlet@samba.org vfs: Fix alternative posix and no-op sys acl implementations to take a mem_ctx is missing in the patchset
Comment 11 Andrew Bartlett 2012-11-20 11:37:32 UTC
So, it all works once that's included?
Comment 12 Andrew Bartlett 2012-11-20 12:22:50 UTC
Created attachment 8214 [details]
ACL fixes for 4.0 including fixes from ambi, and including the AIX build fix
Comment 13 Stefan Metzmacher 2012-11-28 14:58:31 UTC
Comment on attachment 8214 [details]
ACL fixes for 4.0 including fixes from ambi, and including the AIX build fix

I'm currently reviewing and testing this. I already found some missing things
and will upload a new patchset once I'm happy with the result.
Comment 14 Stefan Metzmacher 2012-12-03 08:19:10 UTC
Created attachment 8249 [details]
Same patches for v4-0-test incl. cherry-pick info (without unrelated "Rework ldap attribute fetch" patch)

See https://bugzilla.samba.org/attachment.cgi?id=8214

Karolin, please pick this to v4-0-test, but please note that
some more patches will follow.
Comment 15 Stefan Metzmacher 2012-12-03 08:21:26 UTC
Created attachment 8250 [details]
Additional patches for v4-0-test (part1)
Comment 16 Karolin Seeger 2012-12-03 08:44:23 UTC
(In reply to comment #14)
> Created attachment 8249 [details]
> Same patches for v4-0-test incl. cherry-pick info (without unrelated "Rework
> ldap attribute fetch" patch)
> 
> See https://bugzilla.samba.org/attachment.cgi?id=8214
> 
> Karolin, please pick this to v4-0-test, but please note that
> some more patches will follow.

Metze, please add me to the CC list if the bug report is not assigned to me.
Otherwise it might happen that I don't get the information in time.

The patch does not apply to v4-0-test:

--- snip ---
Applying: selftest: check that samba-tool gpo works for basic operations
error: patch failed: source4/selftest/tests.py:405
error: source4/selftest/tests.py: patch does not apply
Patch failed at 0032 selftest: check that samba-tool gpo works for basic operations
--- snap ---
Comment 17 Stefan Metzmacher 2012-12-03 08:46:56 UTC
(In reply to comment #14)
> Created attachment 8249 [details]
> Same patches for v4-0-test incl. cherry-pick info (without unrelated "Rework
> ldap attribute fetch" patch)
> 
> See https://bugzilla.samba.org/attachment.cgi?id=8214
> 
> Karolin, please pick this to v4-0-test, but please note that
> some more patches will follow.

This depends on https://attachments.samba.org/attachment.cgi?id=8248
from https://bugzilla.samba.org/show_bug.cgi?id=9121
Comment 18 Stefan Metzmacher 2012-12-03 10:10:20 UTC
Created attachment 8255 [details]
Additional patches for v4-0-test (part2)
Comment 19 Michael Adam 2012-12-03 11:30:42 UTC
Comment on attachment 8250 [details]
Additional patches for v4-0-test (part1)

ACK
Comment 20 Michael Adam 2012-12-03 11:34:34 UTC
Comment on attachment 8255 [details]
Additional patches for v4-0-test (part2)

ACK
Comment 21 Michael Adam 2012-12-03 11:36:22 UTC
==> Karolin for 4.0
Comment 22 Karolin Seeger 2012-12-03 12:17:46 UTC
Pushed to autobuild-v4-0-test.
Comment 23 Karolin Seeger 2012-12-03 19:32:43 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!