Re: [PATCH] Minor sys_umount fix (changes semantics slightly)

Malcolm Beattie (mbeattie@sable.ox.ac.uk)
Wed, 15 Dec 1999 17:41:03 +0000 (GMT)


Guest section DW writes:
> On Wed, Dec 15, 1999 at 12:46:10PM +0000, Malcolm Beattie wrote:
> > Here's a one-letter patch for 2.3.x to sys_umount to allow it to
> > unmount filesystems whose root inode is a symlink.
> >
> > However, this changes umount semantics very slightly: currently, with
> > a 2.2 kernel, if you have a symlink /foo to /some/mountpoint then
> > umount("/foo") will unmount /some/mountpoint but after this patch
> > that will no longer work. If this inconveniences anyone greatly,
> > please scream
>
> Hmm. I think the standards say "If the directory on which a file system
> is to be mounted is a symlink, the fs is mounted on the dir to which
> the symlink refers" - will check later.

I can't find any mention of mount(8) or mount(2) in SUSv2 or POSIX.1.

> Also, think of the very common case where people mount a CDROM
> on /dev/cdrom which is a symlink to /dev/sr0 or /dev/hdc or so.

Good point. After coming up with a patch which solves this, I then
discovered that umount(8) already follows symlinks in userland
before it calls umount(2) (actually oldumount(2)). That means that
mount(8) works fine with my original patch even for symlinks like
/dev/cdrom. It also works OK with mine because if I do
mount -t mls ... /tmp/mls
then readlink on /tmp/mls produces "[MLS]" while follow_link does
the magic of redirection. umount(8) tried to find /tmp/[MLS], fails
with ENOENT and then goes and ahead and calls oldumount(2) on
"/tmp/mls", trusting that the caller knows what they're doing.

If having the system call itself follow symlinks where appropriate
is considered absolutely necessary (rather than leaving it up to
userland) then the following patch solves it, but it looks to me as
though the kernel is cleaner if it just assumes the caller passes in
the right name to start with.

The patch below starts by doing an lnamei and then checks if the
resulting symlink is indeed a mountpoint itself. If not, then it does
an ordinary namei to follow any potential link and reproduce the old
behaviour. This one's against 2.2 but there's no significant
difference between that and 2.3 as far as this bit goes.

------------------------------ cut here ------------------------------
--- fs/super.c.old Wed Dec 15 17:40:53 1999
+++ fs/super.c Wed Dec 15 16:58:04 1999
@@ -768,6 +768,14 @@
* unixes. Our API is identical to OSF/1 to avoid making a mess of AMD
*/

+static __inline__ struct super_block *mount_point_sb(struct inode *inode)
+{
+ struct super_block *sb = inode->i_sb;
+ if (sb && inode == sb->s_root->d_inode)
+ return sb;
+ return 0;
+}
+
asmlinkage int sys_umount(char * name, int flags)
{
struct dentry * dentry;
@@ -778,6 +786,11 @@

lock_kernel();
dentry = lnamei(name);
+ if (!IS_ERR(dentry) && !mount_point_sb(dentry->d_inode)) {
+ /* try following symlink if present */
+ dput(dentry);
+ dentry = namei(name);
+ }
retval = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
struct inode * inode = dentry->d_inode;
@@ -788,9 +801,9 @@
if (IS_NODEV(inode))
retval = -EACCES;
} else {
- struct super_block *sb = inode->i_sb;
+ struct super_block *sb = mount_point_sb(inode);
retval = -EINVAL;
- if (sb && inode == sb->s_root->d_inode) {
+ if (sb) {
dev = sb->s_dev;
retval = 0;
}
------------------------------ cut here ------------------------------

--Malcolm

-- 
Malcolm Beattie <mbeattie@sable.ox.ac.uk>
Unix Systems Programmer
Oxford University Computing Services

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/