Bug 221081 - {file,dir}_mode bug on CIFS shares with unix_extensions=no
{file,dir}_mode bug on CIFS shares with unix_extensions=no
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: kernel (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jeff Layton
Brian Brock
bzcl34nup
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-01 08:19 EST by Andrew Zabolotny
Modified: 2014-06-18 03:35 EDT (History)
8 users (show)

See Also:
Fixed In Version: 2.6.26
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-29 09:31:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
proposed patch -- don't override the mode when unix extensions aren't enabled (904 bytes, patch)
2007-09-21 09:05 EDT, Jeff Layton
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
Samba Project 4472 None None None Never

  None (edit)
Description Andrew Zabolotny 2007-01-01 08:19:42 EST
Description of problem:

file_mode and dir_mode options of CIFS filesystem driver don't completely work
as expected for newly-created files or directories on shares that have
"unix_extensions=no".

Version-Release number of selected component (if applicable):

3.0.23c

How reproducible:

Always

Steps to Reproduce:
1. Add "unix extensions = no" to the global section in /etc/samba/smb.conf
2. Mount the share like this: "mount -t cifs //host/share /mnt/tmp -o
file_mode=0666,dir_mode=0777,username=xxx" and enter the correct password.
3. Ensure that all files on the mount are mode 666 and all dirs are 777, as
expected. Ensure your umask is 0022.
4. Go into a directory with real (not faked by cifs.ko) write permissions and
execute: "mkdir test; ls -lda test; sleep 1; ls -lda test"
  
Actual results:

[2|root@zap|/mnt/tmp/incoming]mkdir test; ls -lda test; sleep 1; ls -lda test
drwxr-xr-x 2 root root 0 Янв  1 16:04 test/
drwxrwxrwx 1 root root 0 Янв  1 16:04 test/

Expected results:

drwxrwxrwx 2 root root 0 Янв  1 16:04 test/
drwxrwxrwx 1 root root 0 Янв  1 16:04 test/

Additional info:

It seems like the 'chmod' operation is not ignored as expected, but somehow gets
cached for some indefinite time (it may last for a very long time). Another test
that gives results similar to above:

[2|root@zap|/mnt/tmp/incoming]chmod 700 test; ls -lad test; sleep 1; ls -lad test
drwx------ 1 root root 0 Янв  1 16:04 test/
drwxrwxrwx 1 root root 0 Янв  1 16:04 test/

This wouldn't be so bad if *both* owner and access mode would be cached, the
problem is that only the access mode is cached, so if you are a regular user you
won't be able to write to the directory you just created! This is quite annoying
if you copy large amounts of files recursively; however this does not always
exhibit, I can't understand when, but on some operations this 'cache' is
guaranteedly flushed, so with some applications recursive copy works fine, with
others (e.g. midnight commander) it doesn't.
Comment 1 Jay Fenlason 2007-01-02 11:28:03 EST
Does this happen when using the CIFS filesystem to mount non-Samba servers? 
Ifso, it's a kernel bug, and should be reassigned to the kernel component.  
Otherwise I'll need a complete /etc/samba/smb.conf or a wireshark capture of 
the entire interaction between the client and server. 
Comment 2 Andrew Zabolotny 2007-01-04 06:03:23 EST
I've tried it today on a Windows share and the bug is still present.
I would reassign it to the kernel component but I couldn't find it in the huge
list, sorry :-)
Comment 3 Jay Fenlason 2007-01-04 10:09:48 EST
Kernel CIFS bug, reassigning. 
Comment 4 Chuck Ebbert 2007-03-26 10:33:26 EDT
Test kernels for this issue are at:

http://people.redhat.com/cebbert

Please test and report back.
Comment 5 Andrew Zabolotny 2007-03-27 12:00:52 EDT
I have tried the kernel-2.6.20-1.2937.fc6.x86_64.rpm package. The bug is still
present:

[2|zap@zap|/mnt/cs-zap]uname -a
Linux zap.ozerki.lan 2.6.20-1.2937.fc6 #1 SMP Sun Mar 25 20:34:49 EDT 2007
x86_64 x86_64 x86_64 GNU/Linux
[2|zap@zap|/mnt/cs-zap]whoami
zap
[2|zap@zap|/mnt/cs-zap]mkdir test; ls -lad test; echo "" >test/file; sleep 1; ls
-lad test; echo "" >test/file; ls -la test
drwxr-xr-x 2 root root 0 Mar 27  2007 test/
bash: test/file: Permission denied
drwxrwxrwx 1 root root 0 Mar 27  2007 test/
total 4
drwxrwxrwx  1 root root 0 Mar 27  2007 ./
drwxrwxrwx 54 root root 0 Mar 27  2007 ../
-rw-r--r--  1 root root 1 Mar 27  2007 file

As you can see in the later listing, the mode on the file is 0644, although
file_mode was 0666 while mounting the share. And it doesn't drop to 0666
whatever I do with it, but if you wait some time it will earlier or later reset
to the correct value.
Comment 6 Jeff Layton 2007-09-20 15:58:15 EDT
I think this is an artifact of the way that inodes are created in this
situation. When inodes are created they get whatever mode the VFS passed to the
call. Eventually though, they get revalidated and then they pick up whatever
file/dir_mode the share is mounted with. I find this confusing too -- perhaps we
can make an argument to get this changed upstream...


Comment 7 Chuck Ebbert 2007-09-20 17:11:54 EDT
I think this may have been fixed. Can reporter try a 2.6.22 FC6 kernel?
Comment 8 Jeff Layton 2007-09-20 18:03:43 EDT
Actually, I don't think this is fixed. I see similar behavior on rawhide, even...

I think I understand what the problem is. When I get some time, I'll see if I
can roll up a patch to fix it.
Comment 9 Jeff Layton 2007-09-20 18:04:59 EDT
This is similar to RHEL5 bug #287401

Comment 10 Andrew Zabolotny 2007-09-21 06:58:07 EDT
I currently use kernel-2.6.22.1-41.fc7 and I'm still fighting every day with
this bug. Quite annoying.
Comment 11 Jeff Layton 2007-09-21 09:05:15 EDT
Created attachment 202051 [details]
proposed patch -- don't override the mode when unix extensions aren't enabled

This patch seems to correct the behavior, and I think it's the right thing to
do here. It just stops the cifs code from overriding the mode when unix
extensions are not enabled.

Not only is this behavior an inconvenience, I think it's potentially a security
issue. For instance, presume that someone has more restrictive permissions on a
cifs share than the typical create mode:

file_mode=0600,dir_mode=0700

...and then the user goes to create a file:

creat("/mnt/cifs/foobar", 0644);

...the user would probably expect that the file would end up with a mode of
0600. There is a window there where it will be 0644, potentially exposing the
data.

Now, you might make the case someone using a mode of 0644 shouldn't expect that
the file doesn't get exposed. That would probably be valid except for the fact
that when unix extensions aren't enabled, the code also does not respect the
umask.

Andrew, could you test this patch and let me know if it seems to fix the issue
for you?
Comment 12 Andrew Zabolotny 2007-12-17 14:25:05 EST
The patch works perfectly, thank you! Tried it on kernel-2.6.23.8-34.fc7.

It's sad the patch didn't get into fedora 8 :(
Comment 13 Jeff Layton 2008-01-09 09:15:38 EST
we're still debating this patch upstream. The problem is that while the current
behavior is a bit goofy, it allows certain test suites to run correctly.
Removing it might cause regressions in some cases.

After discussing it with Steve French, he recommended that we wait until the
unix mode to windows ACL mapping patches are in place. A lot of that is upstream
now, so I need to dust off the above patch and see what needs to be done at this
point and what changing this would actually break.
Comment 14 Jeff Layton 2008-01-09 15:32:20 EST
Reposted patch upstream on linux-cifs-client@lists.samba.org. Awaiting comments...
Comment 15 Jeff Layton 2008-01-23 13:07:46 EST
Adding Steve in case he wants to see the original bug report on this...
Comment 16 Bug Zapper 2008-04-04 01:26:24 EDT
Fedora apologizes that these issues have not been resolved yet. We're
sorry it's taken so long for your bug to be properly triaged and acted
on. We appreciate the time you took to report this issue and want to
make sure no important bugs slip through the cracks.

If you're currently running a version of Fedora Core between 1 and 6,
please note that Fedora no longer maintains these releases. We strongly
encourage you to upgrade to a current Fedora release. In order to
refocus our efforts as a project we are flagging all of the open bugs
for releases which are no longer maintained and closing them.
http://fedoraproject.org/wiki/LifeCycle/EOL

If this bug is still open against Fedora Core 1 through 6, thirty days
from now, it will be closed 'WONTFIX'. If you can reporduce this bug in
the latest Fedora version, please change to the respective version. If
you are unable to do this, please add a comment to this bug requesting
the change.

Thanks for your help, and we apologize again that we haven't handled
these issues to this point.

The process we are following is outlined here:
http://fedoraproject.org/wiki/BugZappers/F9CleanUp

We will be following the process here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping to ensure this
doesn't happen again.

And if you'd like to join the bug triage team to help make things
better, check out http://fedoraproject.org/wiki/BugZappers
Comment 17 Andrew Zabolotny 2008-04-04 16:23:07 EDT
Bug is still present with latest kernel (kernel-2.6.24.4-64.fc8).
Comment 18 Jeff Layton 2008-04-04 16:28:22 EDT
Yep. There's still some reservations about this patch upstream by the
maintainer, though I still think that it's the right thing to do. I also have
some other somewhat related patches that I'd like to see go in as well. I'll
plan to roll them up in a set and propose them when I get some time.

Feel free to chime in on linux-cifs-client@lists.samba.org if you wish.
Comment 19 Jeff Layton 2008-04-29 09:44:08 EDT
I'm working on a new patchset for this that includes changes to make
cifs_setattr silently ignore ownership and mode changes that don't affect
ATTR_READONLY.

I've also got a related reproducer that shows that in-memory mode changes like
this can be lost as soon as the box is subject to memory pressure.

I need to do a bit more testing and cleanup on the patches. I'll try to post
them here and upstream in the next few days.
Comment 20 Jeff Layton 2008-05-05 15:23:28 EDT
New patchset sent upstream. Awaiting comments...
Comment 21 Jeff Layton 2008-05-19 16:56:15 EDT
After working with Steve upstream, I think we have a resolution for this. I sent
him a respun patchset today and hopefully it'll be committed soon...

This patchset does a good job of making sure that we don't allow ephemeral mode
changes, but if someone turns out to need them, then there is a new "dynperm"
mount option that allows it. It also extends the "setuids" mount option to allow
chown to change the ownership of the file if and only if that's enabled when
unix extensions aren't on.

AFAIK, the patchset is acceptible and is just waiting for him to get time to
review and merge it.
Comment 22 Jeff Layton 2008-05-27 06:50:42 EDT
Patchset has been merged into Steve F's cifs-2.6 git tree. Barring major
problems, it should go into 2.6.27. I'll leave this case open until the patchset
has had more soak time.
Comment 23 Chuck Ebbert 2008-05-30 00:09:42 EDT
in 2.6.26 as:
Commit:     d0a9c078db4769f7305ff9774558776d12bfb25b
Comment 24 Jeff Layton 2008-05-30 06:49:31 EDT
Unfortunately, that patch isn't complete. Steve pushed the patch that adds the
new mount option, but the patches to change the default behavior and to make the
mount option actually do anything are still queued for 2.6.27.

I'm going to reopen this since it's not actually in rawhide yet...
Comment 25 Jeff Layton 2008-07-29 09:31:09 EDT
Actually...looks like steve pushed the rest of the patches for this into 2.6.26
late. So this should now be fixed in 2.6.26 kernels.

Note You need to log in before you can comment on or make changes to this bug.