Bug 6535 - Error in umount.cifs
Error in umount.cifs
Status: RESOLVED WONTFIX
Product: Samba 3.4
Classification: Unclassified
Component: Client Tools
3.4.0
Other Linux
: P3 normal
: ---
Assigned To: Jeff Layton
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-06 09:51 UTC by Peter Gordon
Modified: 2011-02-01 13:01 UTC (History)
1 user (show)

See Also:


Attachments
strace of umount.cifs unreachable_share (11.82 KB, text/plain)
2009-09-23 19:52 UTC, Mike Perrin
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Gordon 2009-07-06 09:51:50 UTC
Let's suppose that I have mount using CIFS, and the server dies.
I would like to be able to umount the mount point.
mount.cifs does a statfs on the mountpoint and, if this mountpoint is not 
available, fails.

Apart from having to wait for the timeout, the umount fails, and there is no way to remove the entry. This makes all the normal programs hang, df, lsof ....
As far as I can tell, assuming that the user is sure that it is a cifs mount 
point, there is no reason to perform this check. 

Calling umount2 directly and changing /etc/mtab is all that is needed.
Maybe the -f flag could be used to avoid the check.


        rc = statfs(mountpoint, &statbuf);

        if(rc || (statbuf.f_type != CIFS_MAGIC_NUMBER)) {
                printf("This utility only unmounts cifs filesystems.\n");
                return -EINVAL;
        }
Comment 1 Mike Perrin 2009-08-17 14:52:58 UTC
My observation is that both version 3.0.28a (Ubuntu 8.04) and 3.3.2 (Ubuntu 9.04) unmount the share after the timeout. The delay is disruptive and annoying, however, and I concur that there is no reason to wait if the mountpoint is identified as type cifs in mtab. Requiring the -f flag to enable the alternative mtab check would be reasonable however.
Comment 2 Jeff Layton 2009-09-19 11:25:14 UTC
If you rename /sbin/umount.cifs so that it's not found by /bin/umount, does this problem still occur?

In the master samba branch, umount.cifs is no longer built by default since it's not really needed, so this may just be a matter of having the distro packagers not ship it.
Comment 3 Mike Perrin 2009-09-19 12:43:35 UTC
Hiding /sbin/umount.cifs reduces the delay but does not eliminate it when unmounting a disconnected share using /bin/umount. Delays with umount.cifs hidden ranged from 18 to 27 seconds in four trials. With umount.cifs exposed, delays of 30 to 53 seconds were observed, also over four trials.

I do not agree that umount.cifs is no longer needed. My share mounting management system, written in shell script, depends on mount.cifs and umount.cifs installed suid to allow automatic mount/unmount of shares for normal users without granting them universal mount privileges.
Comment 4 Jeff Layton 2009-09-19 13:09:34 UTC
If you're dependent upon making umount.cifs setuid root, then you'll also probably need to build mount.cifs with its legacy setuid behavior enabled. Personally, I don't recommend it since you're granting unprivileged users more permission than they should have, but its your environment.

Care to propose a patch with what you'd like to see umount.cifs do? That'll give us a concrete starting point for discussion.
Comment 5 Mike Perrin 2009-09-19 13:37:08 UTC
I haven't looked at the source code (my C experience is little and long ago), I simply made use of the suid behavior of *mount.cifs in the distros I developed my scripts on: Ubuntu 8.04 and 9.04, Fedora 10 and 11, Slackware 12.2 and 13.0, and PCLinuxOS 2009.2. It seems to me that the code suggested by Peter Gordon in his original post is a good place to start. I think his point is valid. If we know it is a cifs filesystem, why do we have to check?
Comment 6 Jeff Layton 2009-09-19 13:44:17 UTC
I'm sorry, I'm a little unclear on this point...how do we know that the filesystem in question is CIFS? It's certainly possible for a user to call umount.cifs directly, so you can't count on /bin/umount to do that for you.
Comment 7 Mike Perrin 2009-09-19 16:54:12 UTC
Perhaps I misinterpreted Peter's original report. I took it to suggest that mtab could be consulted to verify a cifs filesystem and allow umount.cifs to proceed without delay if the server was unreachable. On review, however, he seems to propose that the -f flag could be used to skip the check entirely based on the user's conviction that it is a cifs filesystem. I am more than unclear, I am now confused. Hopefully Peter will weigh in and clarify his intent.
Comment 8 Peter Gordon 2009-09-21 01:01:58 UTC
The main problem still remains the same.
Add a CIFS mount point and some time after, have the server go down and issue a umount.

Umounting
=========
I don't want to check the state of the server, as it seems to be irrelevant. 
The mount point can be found from the file /etc/mtab, so why not just rely on that? Do the umount2 and change the mtab accordingly.

The -f flag
===========

Assuming that umount.cifs issues a umount2 call and then changes /etc/mtab, there is no need for a -f flag. 

If however, the /etc/mtab file gets changed first, and the program is aborted before it gets to the umount2 command, the next time umount.cifs is run, the type of the filesystem will not be able to be verified, so a -f flag would be useful.

I hope that I haven't muddied the waters still further.




Comment 9 Jeff Layton 2009-09-22 07:57:33 UTC
I'm a little leery of trusting the mtab to that degree. It's often the case that the information in it is incomplete or wrong. umount.cifs should probably be fixed to not change the mtab unless the umount actually succeeds too...

I also wouldn't want to trust the "-f" flag in the fashion you suggest for anyone but the actual root user. Allowing any user to just pass "-f" and unmount any filesystem seems a bit dangerous.

There is another way to determine the fstype for the mount. If /proc/self/mounts exists, then you could walk it instead of the mtab to determine it. That's a bit more difficult to spoof than the mtab.

That should (in principal) get around a statfs call. Before you go down that path though, it might be best to post an strace of the umount.cifs call with timing info so we can determine how long each syscall is actually taking. i.e.:

# strace -f -T -tt -o /tmp/umount.strace /sbin/umount.cifs /path/to/dead/cifs/mount

...then attach /tmp/umount.strace to this BZ.
Comment 10 Mike Perrin 2009-09-23 19:52:09 UTC
Created attachment 4733 [details]
strace of umount.cifs unreachable_share

I had to run sudo. When run as user I got:
mike@stealthwork:~$ strace -f -T -tt -o /tmp/umount.strace /sbin/umount.cifs /home/mike/Public
Trying to unmount when /sbin/umount.cifs not installed suid

Although umount.cifs was in fact set suid:
mike@stealthwork:~$ ls -l /sbin/umount.cifs
-rwsr-sr-x 1 root root 14000 2009-07-08 00:28 /sbin/umount.cifs

I mention this in case it might have affected any of the timings.
Comment 11 Karolin Seeger 2010-05-26 08:10:40 UTC
Updating component.
Comment 12 Jeff Layton 2011-02-01 13:01:53 UTC
Since cifs-utils no longer ships this tool, I'm closing this bug as WONTFIX. Please reopen if you wish to discuss it further.