In Samba 3.6, "open for execution" was successful even if the user had no execute permissions. In Samba 4.0 this was fixed by doing a proper ACL-check against the provided access_mask. While this is correct, this change in behaviour is a potential problem for those migrating their fileserver from Samba 3.6 (or older) to Samba 4.0 (or newer), since they need to audit their files for missing x-bits in acls/permissions... An option is needed to re-establish the old behaviour in order to ease migrations.
Created attachment 9202 [details] patchset for master This patchset has been pushed to autobuild for master.
Comment on attachment 9202 [details] patchset for master The patchset applies cleanly to v4-1-test. So once it has landed in master, the patches can be cherry-picked.
Comment on attachment 9202 [details] patchset for master asking for review for this patchset to be applied to v4-1-test
Created attachment 9203 [details] patchset for v4-0-test Patchset for 4.0: There is one tiny difference in context here, hence the extra patchset. If we want this for 4.0, we should redo this with cherry-pick-info once code has landed in master. Do we want it for 4.0?
The patches have now gone into master as: de3bc10ef69f23e7dab9fc3f6990bb403824b14e 1e29d730663382875d96c275c60e022a1c33a2d1 a2a3c9f36d7a19d75924cff25fa1b450d85ee6d6 Karo: for 4.1, these can be cherry-picked in this order. For 4.0, would you like me to redo the ACKed patch with cherry-pick-info? Cheers - Michael
(In reply to comment #5) > Created attachment 9203 [details] > patchset for v4-0-test > > Patchset for 4.0: > There is one tiny difference in context here, hence the extra patchset. > If we want this for 4.0, we should redo this with cherry-pick-info once code > has landed in master. > > Do we want it for 4.0? We need to discuss it, because usually, we do not add new parameters in minor releases. But I think in this case, it makes sense. Are there any objections?
(In reply to comment #7) > (In reply to comment #5) > > Created attachment 9203 [details] [details] > > patchset for v4-0-test > > > > Patchset for 4.0: > > There is one tiny difference in context here, hence the extra patchset. > > If we want this for 4.0, we should redo this with cherry-pick-info once code > > has landed in master. > > > > Do we want it for 4.0? > > We need to discuss it, because usually, we do not add new parameters in minor > releases. But I think in this case, it makes sense. > > Are there any objections? I am in favour of adding the parameter. For those who would like to comment, here is the argument again: The reason for adding the parameter is just that an upgrade from 3.6 or earlier to a 4.0 without this switch can present bad traps for the user.
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > Created attachment 9203 [details] [details] [details] > > > patchset for v4-0-test > > > > > > Patchset for 4.0: > > > There is one tiny difference in context here, hence the extra patchset. > > > If we want this for 4.0, we should redo this with cherry-pick-info once code > > > has landed in master. > > > > > > Do we want it for 4.0? > > > > We need to discuss it, because usually, we do not add new parameters in minor > > releases. But I think in this case, it makes sense. > > > > Are there any objections? > > I am in favour of adding the parameter. > > For those who would like to comment, here is the argument again: > > The reason for adding the parameter is just that > an upgrade from 3.6 or earlier to a 4.0 without this switch > can present bad traps for the user. We could turn this into an undocumented parametric smbd:compat parameter. This will avoid the problem that we can not add new documented parameters within a release stream.
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > (In reply to comment #5) > > > > Created attachment 9203 [details] [details] [details] [details] > > > > patchset for v4-0-test > > > > > > > > Patchset for 4.0: > > > > There is one tiny difference in context here, hence the extra patchset. > > > > If we want this for 4.0, we should redo this with cherry-pick-info once code > > > > has landed in master. > > > > > > > > Do we want it for 4.0? > > > > > > We need to discuss it, because usually, we do not add new parameters in minor > > > releases. But I think in this case, it makes sense. > > > > > > Are there any objections? > > > > I am in favour of adding the parameter. > > > > For those who would like to comment, here is the argument again: > > > > The reason for adding the parameter is just that > > an upgrade from 3.6 or earlier to a 4.0 without this switch > > can present bad traps for the user. > > We could turn this into an undocumented parametric smbd:compat parameter. This > will avoid the problem that we can not add new documented parameters within a > release stream. I prefer documented parameters.
(In reply to comment #10) > I prefer documented parameters. Me too. But as you pointed out, we can't add parameters within a release stream. So should we better leave this as a recommended patch somewhere on the website for people who hit this?
(In reply to comment #11) > (In reply to comment #10) > > I prefer documented parameters. > > Me too. But as you pointed out, we can't add parameters within a release > stream. So should we better leave this as a recommended patch somewhere on the > website for people who hit this? Volker, there is a subtle but important difference between "usually, we do not ..." as Karolin has put it and your "we can't" which Karolin has not written. :-) I think we should not make it unnecessarily complicated. Either we add the proper option or we don't. Cheers - Michael
(In reply to comment #11) > (In reply to comment #10) > > I prefer documented parameters. > > Me too. But as you pointed out, we can't add parameters within a release > stream. So should we better leave this as a recommended patch somewhere on the > website for people who hit this? Of course we can addit, but we need to agree on this. And if there are no objections, I am happy to add it, because it makes sense in this case. I just wanted to ask before skipping the commitment...
+1 on adding the parameter. Rules are not absolute, and this helps the upgrade path for users. Users are kings (and queens of course :-), without them we have no project :-). Jeremy.
(In reply to comment #6) > The patches have now gone into master as: > > de3bc10ef69f23e7dab9fc3f6990bb403824b14e > 1e29d730663382875d96c275c60e022a1c33a2d1 > a2a3c9f36d7a19d75924cff25fa1b450d85ee6d6 > > Karo: for 4.1, these can be cherry-picked in this order. > > For 4.0, would you like me to redo the ACKed patch with cherry-pick-info? > > Cheers - Michael No, thanks. I added the hashes to the latest commit. I think that's sufficient.
(In reply to comment #14) > +1 on adding the parameter. Rules are not absolute, and this helps the upgrade > path for users. Users are kings (and queens of course :-), without them we have > no project :-). > Jeremy. Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Created attachment 9206 [details] docs: Fix typos Follow-up patch to fix minor typos in the man page introduced by a2a3c9f36d (docs: document "acl allow execute always").
(In reply to comment #17) > Created attachment 9206 [details] > docs: Fix typos > > Follow-up patch to fix minor typos in the man page introduced by a2a3c9f36d > (docs: document "acl allow execute always"). Note: Patch not yet in master, sending to samba-technical first...
Comment on attachment 9206 [details] docs: Fix typos I just pushed the corresponding patch to master. ACK from me to cherry-pick it for the release!
(In reply to comment #19) > Comment on attachment 9206 [details] > docs: Fix typos > > I just pushed the corresponding patch to master. > > ACK from me to cherry-pick it for the release! 4af7b709e in master, cherry-picked and pushed to autobuild-v4-1-test.
(In reply to comment #20) > (In reply to comment #19) > > Comment on attachment 9206 [details] [details] > > docs: Fix typos > > > > I just pushed the corresponding patch to master. > > > > ACK from me to cherry-pick it for the release! > > 4af7b709e in master, cherry-picked and pushed to autobuild-v4-1-test. and pushed to autobuild-v4-0-test.
Pushed to v4-0-test and v4-1-test. Leaving bug report open as a reminder to add something to the 4.0 release notes.
(In reply to comment #22) > Pushed to v4-0-test and v4-1-test. > Leaving bug report open as a reminder to add something to the 4.0 release > notes. Added hint to the release notes. Closing out bug report. Thanks!