Bug 8191 - vfs_gpfs dosn't honor ACE_FLAG_INHERITED_ACE
vfs_gpfs dosn't honor ACE_FLAG_INHERITED_ACE
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: VFS Modules
3.6.0rc1
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 14:04 UTC by Gregor Beck (550 Unknown user)
Modified: 2012-05-23 20:40 UTC (History)
1 user (show)

See Also:


Attachments
patchset for v3-6-test (3.00 KB, patch)
2011-05-31 14:15 UTC, Gregor Beck (550 Unknown user)
obnox: review+
jra: review+
Details
Jeremy's add-on patch (4.93 KB, patch)
2011-06-01 08:14 UTC, Michael Adam
obnox: review+
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gregor Beck (550 Unknown user) 2011-05-31 14:04:18 UTC
vfs_gpfs masks out the flag ACE_FLAG_INHERITED_ACE supported by gpfs
Comment 1 Gregor Beck (550 Unknown user) 2011-05-31 14:15:56 UTC
Created attachment 6503 [details]
patchset for v3-6-test
Comment 2 Jeremy Allison 2011-05-31 20:12:31 UTC
Comment on attachment 6503 [details]
patchset for v3-6-test

I *hate* this patch (sorry :-). I'm changing it for master and will add an updated version for 3.6.0.

Directly manipulating bits in the ACE mask using a mix of SEC_ACE_XXX and SMB_ACE4_XXX flags is a recipe for disaster later on.

I'm factoring this out to 2 functions which make the bit manipulations clearer.

Jeremy.
Comment 3 Michael Adam 2011-05-31 20:14:40 UTC
Comment on attachment 6503 [details]
patchset for v3-6-test

ACK
Comment 4 Jeremy Allison 2011-05-31 20:15:47 UTC
Sorry nack on this patch as-is. Please wait for my 3.6.0 update.

Jeremy.
Comment 5 Michael Adam 2011-05-31 20:24:31 UTC
Comment on attachment 6503 [details]
patchset for v3-6-test

sorry, jeremy, I missed your comment
Comment 6 Michael Adam 2011-05-31 20:39:30 UTC
(In reply to comment #2)
> Comment on attachment 6503 [details]
> patchset for v3-6-test
> 
> I *hate* this patch (sorry :-). I'm changing it for master and will add an
> updated version for 3.6.0.
> 
> Directly manipulating bits in the ACE mask using a mix of SEC_ACE_XXX and
> SMB_ACE4_XXX flags is a recipe for disaster later on.

I don't really understand your objection here:
There is no mix of SEC_ACE and SMB_ACE4 flags used directly.
It is just a mapping from SEC_ACE to SMB_ACE4 and vice versa.

I do agree that for the sake of clarity, it might be useful to
create mapping functions for these two directions.

Cheers - Michael
 
> I'm factoring this out to 2 functions which make the bit manipulations clearer.
Comment 7 Michael Adam 2011-05-31 21:08:33 UTC
Summing up an irc discussion:

* Gregor's patches are ok, but the original mapping code is hacky,
  since it simpy assigns SMB_ACE4 flags to SEC_ACE flags (and vice
  versa), which works by coincidence since the old supported flags
  were 1:1 bit-wise.

* Jeremy is working on an add-on patch that will provide proper
  mapping functions for the ace flag bits.
  This patch will be added as a second part of the bugfix.

Cheers - Michael
Comment 8 Michael Adam 2011-05-31 21:09:40 UTC
Comment on attachment 6503 [details]
patchset for v3-6-test

re-ACKing this first patch after our discussion. Adding Jeremy as second reviewer. And waiting for Jeremy's add-on patch.
Comment 9 Michael Adam 2011-05-31 21:10:56 UTC
the review flags confuse me
Comment 10 Jeremy Allison 2011-05-31 23:54:32 UTC
Waiting for autobuild....
Comment 11 Michael Adam 2011-06-01 08:14:16 UTC
Created attachment 6507 [details]
Jeremy's add-on patch

This is Jeremy's add-on patch that has landed in master.
I would like to have this in 3.6 as well, since it greatly
improves the code.
Comment 12 Michael Adam 2011-06-01 08:15:14 UTC
Jeremy, could you review (and ACK) both patches, and hand the bug over to Karolin? Thanks!
Comment 13 Michael Adam 2011-06-01 08:16:05 UTC
Comment on attachment 6507 [details]
Jeremy's add-on patch

Requesting review from jeremy for inclusion into 3.6.0

btw. this patch applies on top of the first patchset.
Comment 14 Jeremy Allison 2011-06-01 22:04:10 UTC
Re-assigning to Karolin to add both to 3.6.0.
Jeremy.
Comment 15 Karolin Seeger 2011-06-02 18:53:44 UTC
Pushed to v3-6-test.
Closing out bug report.

Thanks!