Bug 8211 - "inherit owner = yes" doesn't interact correctly with "inherit permissions = yes" and POSIX ACLs
Summary: "inherit owner = yes" doesn't interact correctly with "inherit permissions = ...
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.5.8
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 7638 7639 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-07 21:41 UTC by Björn Jacke
Modified: 2011-06-15 21:19 UTC (History)
1 user (show)

See Also:


Attachments
git-am fix for 3.5.9. (4.54 KB, patch)
2011-06-08 19:43 UTC, Jeremy Allison
no flags Details
git-am fix for 3.6.0 (5.19 KB, patch)
2011-06-08 20:02 UTC, Jeremy Allison
no flags Details
Updated fix for 3.5.9 - fixes issue Bjorn found. (5.72 KB, patch)
2011-06-08 21:25 UTC, Jeremy Allison
no flags Details
Updated fix for 3.6.0 final - fixes issue Bjorn found. (5.41 KB, patch)
2011-06-08 21:27 UTC, Jeremy Allison
no flags Details
git-am fix for 3.5.9 (7.62 KB, patch)
2011-06-08 21:40 UTC, Jeremy Allison
bjacke: review+
Details
git-am fix for 3.6.0-final (7.31 KB, patch)
2011-06-08 21:40 UTC, Jeremy Allison
bjacke: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Jacke 2011-06-07 21:41:08 UTC
this is a spin-off from the discussion in https://bugzilla.samba.org/show_bug.cgi?id=8083#c18

"inherit owner = yes" doesn't work with directories if "inherit permissions = yes" is being used at the same time.

I tested a simple share with

inherit acls = yes
inherit permissions = yes
inherit owner = yes
read only = no

created files DO inherit the owner of the parent directory but created directories are being owned by the user who created it, which is not intended.
Comment 1 Jeremy Allison 2011-06-07 23:57:56 UTC
Reproduced this.

Just pushed fix for this for master - once it comes back from autobuild I'll attach it for 3.6.0final, and create a version for 3.5.9.

Jeremy.
Comment 2 Jeremy Allison 2011-06-08 19:43:44 UTC
Created attachment 6543 [details]
git-am fix for 3.5.9.
Comment 3 Jeremy Allison 2011-06-08 20:02:01 UTC
Created attachment 6544 [details]
git-am fix for 3.6.0

Fix for 3.6.0-final.

Bjorn please test these patches (they both work in my testing here) and give approval if they're good.

Thanks !

Jeremy.
Comment 4 Björn Jacke 2011-06-08 21:03:34 UTC
hm, on a quick test it still doesn't work in my test environment with directories. But the logs have a good hint:

[2011/06/08 22:51:52.702757,  0] smbd/open.c:321(change_dir_owner_to_parent)
  change_dir_owner_to_parent: device/inode/mode on directory sub1/sub2/Neuer Ordner (6) changed. Refusing to chown !
Comment 5 Jeremy Allison 2011-06-08 21:25:10 UTC
Created attachment 6546 [details]
Updated fix for 3.5.9 - fixes issue Bjorn found.

Try this - should fix the issue you found - we don't need to be checking the mode bits as well as the dev/ino to ensure we're in the same place.
Comment 6 Jeremy Allison 2011-06-08 21:27:58 UTC
Created attachment 6547 [details]
Updated fix for 3.6.0 final - fixes issue Bjorn found.
Comment 7 Jeremy Allison 2011-06-08 21:40:02 UTC
Created attachment 6548 [details]
git-am fix for 3.5.9

This is what I've pushed to master. Should completely fix all correctness issues around ensuring the returned stat struct in an open fsp is correct.
Comment 8 Jeremy Allison 2011-06-08 21:40:49 UTC
Created attachment 6549 [details]
git-am fix for 3.6.0-final

This is what I've pushed to master. Should completely fix all correctness
issues around ensuring the returned stat struct in an open fsp is correct.
Comment 9 Björn Jacke 2011-06-08 22:37:53 UTC
Comment on attachment 6548 [details]
git-am fix for 3.5.9

looks good and finally fixes it for me, thanks a lot!
Comment 10 Björn Jacke 2011-06-09 09:00:34 UTC
reassign to Karolin for inclusion in 3.5 and 3.6
Comment 11 Karolin Seeger 2011-06-09 18:02:37 UTC
Pushed to v3-6-test.

Unfortunately, it's too late for 3.5.9.
Jeremy, do we need to delay 3.5.9 due to this one?
Comment 12 Jeremy Allison 2011-06-09 18:14:06 UTC
Hmm - tough call. How long a delay would you need ?
Comment 13 Karolin Seeger 2011-06-09 18:24:07 UTC
(In reply to comment #12)
> Hmm - tough call. How long a delay would you need ?

The policy is to freeze the branch one week before a bugfix release.
Pushing the patches today would mean to be able to ship on June 16 (instead of June 14). So it's only two days later.

If the patches are 100% uncritical, we can discuss if a exception can be done.
Comment 14 Karolin Seeger 2011-06-09 18:25:45 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > Hmm - tough call. How long a delay would you need ?
> 
> The policy is to freeze the branch one week before a bugfix release.
> Pushing the patches today would mean to be able to ship on June 16 (instead of
> June 14). So it's only two days later.
> 
> If the patches are 100% uncritical, we can discuss if a exception can be done.

                                                       ^^^
                                                       an
Comment 15 Jeremy Allison 2011-06-09 18:33:29 UTC
2 days slip is ok by me. These features just don't work without these patches, and it's something we've had users complaining about in the past. Björn, can you also comment please ?
Comment 16 Björn Jacke 2011-06-09 20:22:53 UTC
I would be fine if the patches would not make it into 3.5.9 if that would delay the release. There are enough other fixes that are important enough to get a 3.5.9 out without extra delays. We can point people complaining about this to the patches - binary package providers can decide on their own whether or not to add the patches into their 3.5.9 packages already.
Comment 17 Jeremy Allison 2011-06-09 22:25:57 UTC
If it's only 2 days, let's just take the hit and ship with the fix included.

Doesn't make sense to leave them out IMHO.

Jeremy.
Comment 18 Karolin Seeger 2011-06-10 19:11:01 UTC
(In reply to comment #17)
> If it's only 2 days, let's just take the hit and ship with the fix included.
> 
> Doesn't make sense to leave them out IMHO.
> 
> Jeremy.

Pushed to v3-5-test.
If there are no objections, I will stick to the schedule nevertheless.
Closing out bug report.

Thanks!
Comment 19 TAKAHASHI Motonobu 2011-06-15 16:02:40 UTC
BUG#7638 is a duplicated.
Comment 20 TAKAHASHI Motonobu 2011-06-15 16:09:44 UTC
(In reply to comment #19)
> BUG#7638 is a duplicated.

BUG#7639 is also a duplicated.
Comment 21 Björn Jacke 2011-06-15 21:18:57 UTC
*** Bug 7639 has been marked as a duplicate of this bug. ***
Comment 22 Björn Jacke 2011-06-15 21:19:42 UTC
*** Bug 7638 has been marked as a duplicate of this bug. ***