Bug 7996 - sgid bit lost on folder rename
Summary: sgid bit lost on folder rename
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: File services (show other bugs)
Version: 3.4.9
Hardware: x86 Linux
: P5 major
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-09 17:12 UTC by Arnaud FLORENT
Modified: 2011-04-01 18:38 UTC (History)
0 users

See Also:


Attachments
git-am fix for 3.5.next (4.37 KB, patch)
2011-03-22 20:46 UTC, Jeremy Allison
no flags Details
git-am fix for 3.5.next (2.68 KB, patch)
2011-03-31 17:51 UTC, Jeremy Allison
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnaud FLORENT 2011-03-09 17:12:04 UTC
on debian lenny system, 

user1 in group1 have a folder folder1 in his home directory owned by group2 whith sgid bit set

drwxrws--- 2 user1 group2 4096 mar  9 18:01 folder1

if this folder is renamed to folder1b via homes share the sgid bit is lost
drwxrwx--- 2 user1 group2 4096 mar  9 18:01 folder1b

this does not happen if the folder is owned by group1


i also tried samba 3.5.6 (backport lenny) and samba 3.5.8 (sernet-samba), but the problem is still there

this completely breaks the sgid effect on the directory for file sharing...
Comment 1 Volker Lendecke 2011-03-09 17:18:00 UTC
You don't happen to have XFS as your file system?

Volker
Comment 2 Arnaud FLORENT 2011-03-09 17:24:12 UTC
no the filesystem is ext3
Comment 3 Arnaud FLORENT 2011-03-10 08:38:45 UTC
and it happens only if user1 is not member of group2
Comment 4 Arnaud FLORENT 2011-03-16 16:36:28 UTC
i have just enable audit module which logs 
chmod ./folder1b mode 0x5f8
0x5f8=2770...
but perms are drwxrwx---
Comment 5 Volker Lendecke 2011-03-18 17:33:16 UTC
Ok, reproduced it.

Jeremy, I think this is essentially a duplicate of 7987. You asked whether that one is reproducable without Windows. This one is. Just set up a share with

chmod 2770

permissions. Log in with smbclient, do a "mkdir foo". Foo will also have the sgid bit set. Do a "ren foo bar", and bar will have lost the sgid bit.

Volker
Comment 6 Arnaud FLORENT 2011-03-18 18:34:32 UTC
after activating "store dos attribute", the sgid bit is not lost after directoy rename.
so there must be a link with the way the dos mode is handled (even if this bit is not theoricaly mapped to any dos attribute)

i tried to have a look at the source but did not really find any explanation...
Comment 7 Jeremy Allison 2011-03-18 23:58:30 UTC
Reproduced in master. I'll take a closer look - thanks !
Jeremy.
Comment 8 Jeremy Allison 2011-03-21 23:46:35 UTC
Testing a fix for this now. More when it passes.
Jeremy.
Comment 9 Jeremy Allison 2011-03-22 20:46:24 UTC
Created attachment 6340 [details]
git-am fix for 3.5.next

Fix for 3.5.next. Volker, please check. I have a more extensive fix for master and 3.6.0.

Jeremy.
Comment 10 Volker Lendecke 2011-03-23 08:53:36 UTC
Question: Why do we have to do any kind of chmod on rename at all? Doing it as root seems very kludgy and dangerous to me?

Volker
Comment 11 Jeremy Allison 2011-03-23 16:09:11 UTC
This chown is for storing the ARCHIVE bit in the unix mode bits, which we need to set on rename. This code path isn't taken if we're storing DOS attributes in an EA, which is why it took so long to track down and reproduce (I normally run with DOS attrs in an EA).

If we're going to do this chown, we either have to note that it obeys the SETGID restriction (i.e. if you're not a group member you'll lose it) or we have to do it as root. Sucks, but them's the breaks :-).

Jeremy.
Comment 12 Jeremy Allison 2011-03-31 17:51:19 UTC
Created attachment 6360 [details]
git-am fix for 3.5.next

After consultation with Volker, don't use the become_root() trick, but simply deny this inside file_set_dosmode().

Jeremy.
Comment 13 Volker Lendecke 2011-04-01 06:34:10 UTC
Comment on attachment 6360 [details]
git-am fix for 3.5.next

Looks good, thanks! The reasons to refuse doing this that we have a workaround with "store dos attributes = yes"
Comment 14 Karolin Seeger 2011-04-01 18:38:09 UTC
Pushed patch to v3-5-test.
Will be included in the next Samba 3.5 bug fix release.
Closing out bug report.

Please feel free to re-open if this needs to be fixed for Samba 3.4 also.

Thanks!