Re: [patch 2/8] allow unprivileged umount
On Fri, 20 Apr 2007 12:25:34 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote:
+static bool permit_umount(struct vfsmount *mnt, int flags)
+{
...
+ return mnt-mnt_uid == current-uid;
+}
Yes, this seems very wrong. I'd have thought that comparing user_struct*'s
would get us a heck of a lot closer to being able to support aliasing of
UIDs between different namespaces.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/8] add user mounts to the kernel
On Fri, 20 Apr 2007 12:25:33 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote:
From: Miklos Szeredi [EMAIL PROTECTED]
Add ownership information to mounts.
A new mount flag, MS_SETUSER is used to make a mount owned by a user.
If this flag is specified, then the owner will be set to the current
real user id and the mount will be marked with the MNT_USER flag. On
remount don't preserve previous owner, and treat MS_SETUSER as for a
new mount. The MS_SETUSER flag is ignored on mount move.
So is a modified mount(8) needed? If so, is there some convenient way
in which testers can get hold of it?
The MNT_USER flag is not copied on any kind of mount cloning:
namespace creation, binding or propagation. For bind mounts the
cloned mount(s) are set to MNT_USER depending on the MS_SETUSER mount
flag. In all the other cases MNT_USER is always cleared.
For MNT_USER mounts a user=UID option is added to /proc/PID/mounts.
This is compatible with how mount ownership is stored in /etc/mtab.
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/fs/namespace.c
===
--- linux.orig/fs/namespace.c 2007-04-20 11:55:02.0 +0200
+++ linux/fs/namespace.c 2007-04-20 11:55:05.0 +0200
@@ -227,6 +227,13 @@ static struct vfsmount *skip_mnt_tree(st
return p;
}
+static void set_mnt_user(struct vfsmount *mnt)
+{
+ BUG_ON(mnt-mnt_flags MNT_USER);
+ mnt-mnt_uid = current-uid;
+ mnt-mnt_flags |= MNT_USER;
+}
I'm a bit surprised to see this. Using uids in-kernel is all rather
old-fashioned and restricted. I'd have expected
mnt-user = get_uid(current-user);
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
On Fri, 20 Apr 2007 12:25:39 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/8] account user mounts
On Fri, 20 Apr 2007 12:25:35 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote:
Add sysctl variables for accounting and limiting the number of user
mounts.
The maximum number of user mounts is set to 1024 by default. This
won't in itself enable user mounts, setting a mount to be owned by a
user is first needed
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/include/linux/sysctl.h
===
--- linux.orig/include/linux/sysctl.h 2007-04-20 11:55:02.0 +0200
+++ linux/include/linux/sysctl.h 2007-04-20 11:55:07.0 +0200
@@ -818,6 +818,8 @@ enum
FS_AIO_NR=18, /* current system-wide number of aio requests */
FS_AIO_MAX_NR=19, /* system-wide maximum number of aio requests */
FS_INOTIFY=20, /* inotify submenu */
+ FS_NR_USER_MOUNTS=21, /* int:current number of user mounts */
+ FS_MAX_USER_MOUNTS=22, /* int:maximum number of user mounts */
FS_OCFS2=988, /* ocfs2 */
Is there a special reason why the enumerated sysctls are needed? We're
trying to get away from using them.
diff -puN include/linux/sysctl.h~unprivileged-mounts-account-user-mounts-fix
include/linux/sysctl.h
--- a/include/linux/sysctl.h~unprivileged-mounts-account-user-mounts-fix
+++ a/include/linux/sysctl.h
@@ -819,8 +819,6 @@ enum
FS_AIO_NR=18, /* current system-wide number of aio requests */
FS_AIO_MAX_NR=19, /* system-wide maximum number of aio requests */
FS_INOTIFY=20, /* inotify submenu */
- FS_NR_USER_MOUNTS=21, /* int:current number of user mounts */
- FS_MAX_USER_MOUNTS=22, /* int:maximum number of user mounts */
FS_OCFS2=988, /* ocfs2 */
};
diff -puN kernel/sysctl.c~unprivileged-mounts-account-user-mounts-fix
kernel/sysctl.c
--- a/kernel/sysctl.c~unprivileged-mounts-account-user-mounts-fix
+++ a/kernel/sysctl.c
@@ -1028,7 +1028,7 @@ static ctl_table fs_table[] = {
#endif
#endif
{
- .ctl_name = FS_NR_USER_MOUNTS,
+ .ctl_name = CTL_UNNUMBERED,
.procname = nr_user_mounts,
.data = nr_user_mounts,
.maxlen = sizeof(int),
@@ -1036,7 +1036,7 @@ static ctl_table fs_table[] = {
.proc_handler = proc_dointvec,
},
{
- .ctl_name = FS_MAX_USER_MOUNTS,
+ .ctl_name = CTL_UNNUMBERED,
.procname = max_user_mounts,
.data = max_user_mounts,
.maxlen = sizeof(int),
_
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/8] allow unprivileged umount
Andrew Morton wrote:
On Fri, 20 Apr 2007 12:25:34 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote:
+static bool permit_umount(struct vfsmount *mnt, int flags)
+{
...
+ return mnt-mnt_uid == current-uid;
+}
Yes, this seems very wrong. I'd have thought that comparing user_struct*'s
would get us a heck of a lot closer to being able to support aliasing of
UIDs between different namespaces.
Not to mention it should be fsuid, not uid.
-hpa
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/8] add user mounts to the kernel
From: Miklos Szeredi [EMAIL PROTECTED]
Add ownership information to mounts.
A new mount flag, MS_SETUSER is used to make a mount owned by a user.
If this flag is specified, then the owner will be set to the current
real user id and the mount will be marked with the MNT_USER flag. On
remount don't preserve previous owner, and treat MS_SETUSER as for a
new mount. The MS_SETUSER flag is ignored on mount move.
So is a modified mount(8) needed? If so, is there some convenient way
in which testers can get hold of it?
I haven't yet added this to mount(8). I'll do that and post patches.
There's an ultra sophisticated mount tester program I use. It's
slightly user unfriendly...
Miklos
#include stdio.h
#include stdlib.h
#include string.h
#include sys/mount.h
int main(int argc, char *argv[])
{
int res;
int un = (strrchr(argv[0], '/') ?: argv[0] - 1)[1] == 'u';
if ((un argc != 3) || (!un argc != 6)) {
fprintf(stderr, usage: %s %s\n, argv[0],
un ? dir flags : dev dir type flags opts);
return 1;
}
if (un)
res = umount2(argv[1], atoi(argv[2]));
else
res = mount(argv[1], argv[2], argv[3], atoi(argv[4]), argv[5]);
if (res == -1) {
perror(argv[0]);
return 1;
}
return 0;
}
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/8] allow unprivileged umount
+static bool permit_umount(struct vfsmount *mnt, int flags)
+{
...
+ return mnt-mnt_uid == current-uid;
+}
Yes, this seems very wrong. I'd have thought that comparing user_struct*'s
would get us a heck of a lot closer to being able to support aliasing of
UIDs between different namespaces.
OK, I'll fix this up.
Actually an earlier version of this patch did use user_struct's but
I'd changed it to uids, because it's simpler. I didn't think about
this being contrary to the id namespaces thing.
Miklos
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
On Fri, 20 Apr 2007 12:25:39 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? V9FS people did express an interest in this. Yeah, I should've CC-ed them, but forgot. Sorry. Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 8/8] allow unprivileged fuse mounts
Use FS_SAFE for fuse fs type, but not for fuseblk. FUSE was designed from the beginning to be safe for unprivileged users. This has also been verified in practice over many years. How does FUSE do this? There are obvious cases like crafting a filesystem which has setuid executables or world-writeable device nodes or whatever. I'm sure there are lots of other cases. Where is FUSE's implementation of all this protection described? Most of it is in Documentation/filesystems/fuse.txt, some of it is code comments. Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? V9FS people did express an interest in this. Yeah, I should've CC-ed them, but forgot. Sorry. And CIFS maybe. They also have an unprivileged mounting suid hack. But I'm not very optimistic about CIFS, seeing some of the code, that's in there. Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/8] mount ownership and unprivileged mount syscall (v4)
This patchset has now been bared to the lowest common denominator that everybody can agree on. Or at least there weren't any objections to this proposal. I would be very glad if this feature can be disabled on compilation. Because this feature is fine for desktops, but not for servers. Another user access to kernel = another security hole. I have mount without setuid on my server. I don't want user access to mount/umount. It needs expicit action from the sysadmin (setting a mount flag), before user mounts are enabled. So if you do nothing it will be exactly as secure as it was before. If you are extra paranoid, you can do echo 0 /proc/sys/fs/max_user_mounts to doubly make sure that user mounts are not enabled ;) Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/8] allow unprivileged umount
On Sat, 21 Apr 2007 10:09:42 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote:
+static bool permit_umount(struct vfsmount *mnt, int flags)
+{
...
+ return mnt-mnt_uid == current-uid;
+}
Yes, this seems very wrong. I'd have thought that comparing user_struct*'s
would get us a heck of a lot closer to being able to support aliasing of
UIDs between different namespaces.
OK, I'll fix this up.
Actually an earlier version of this patch did use user_struct's but
I'd changed it to uids, because it's simpler.
OK..
I didn't think about
this being contrary to the id namespaces thing.
Well I was madly assuming that when serarate UID namespaces are in use, UID
42 in container A will have a different user_struct from UID 42 in
container B. I'd suggest that we provoke an opinion from Eric co before
you do work on this.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/8] mount ownership and unprivileged mount syscall (v4)
This patchset has now been bared to the lowest common denominator that everybody can agree on. Or at least there weren't any objections to this proposal. I would be very glad if this feature can be disabled on compilation. Because this feature is fine for desktops, but not for servers. Another user access to kernel = another security hole. I have mount without setuid on my server. I don't want user access to mount/umount. Andrew, please consider it for -mm. Thanks, Miklos v3 - v4: - simplify interface as much as possible, now only a single option (user=UID) is used to control everything - no longer allow/deny mounting based on file/directory permissions, that approach does not always make sense This patchset adds support for keeping mount ownership information in the kernel, and allow unprivileged mount(2) and umount(2) in certain cases. The mount owner has the following privileges: - unmount the owned mount - create a submount under the owned mount The sysadmin can set the owner explicitly on mount and remount. When an unprivileged user creates a mount, then the owner is automatically set to the user. The following use cases are envisioned: 1) Private namespace, with selected mounts owned by user. E.g. /home/$USER is a good candidate for allowing unpriv mounts and unmounts within. 2) Private namespace, with all mounts owned by user and having the nosuid flag. User can mount and umount anywhere within the namespace, but suid programs will not work. 3) Global namespace, with a designated directory, which is a mount owned by the user. E.g. /mnt/users/$USER is set up so that it is bind mounted onto itself, and set to be owned by $USER. The user can add/remove mounts only under this directory. The following extra security measures are taken for unprivileged mounts: - usermounts are limited by a sysctl tunable - force nosuid,nodev mount options on the created mount -- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- Miloslav Majkls Semler - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/8] allow unprivileged umount
Andrew Morton [EMAIL PROTECTED] writes:
On Sat, 21 Apr 2007 10:09:42 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote:
+static bool permit_umount(struct vfsmount *mnt, int flags)
+{
...
+return mnt-mnt_uid == current-uid;
+}
Yes, this seems very wrong. I'd have thought that comparing user_struct*'s
would get us a heck of a lot closer to being able to support aliasing of
UIDs between different namespaces.
OK, I'll fix this up.
Actually an earlier version of this patch did use user_struct's but
I'd changed it to uids, because it's simpler.
OK..
I didn't think about
this being contrary to the id namespaces thing.
Well I was madly assuming that when serarate UID namespaces are in use, UID
42 in container A will have a different user_struct from UID 42 in
container B. I'd suggest that we provoke an opinion from Eric co before
you do work on this.
That is what I what I have been thinking as well, storing a user
struct on each mount point seems sane, plus it allows per user mount
rlimits which is major plus. Especially since we seem to be doing
accounting only for user mounts a per user rlimit seems good.
To get the user we should be user fs_uid as HPA suggested.
Finally I'm pretty certain the capability we should care about in
this context is CAP_SETUID. Instead of CAP_SYS_ADMIN.
If we have CAP_SETUID we can become which ever user owns the mount,
and the root user in a container needs this, so he can run login
programs. So changing the appropriate super user checks from
CAP_SYS_ADMIN to CAP_SETUID I think is the right thing todo.
With the CAP_SETUID thing handled I'm not currently seeing any adverse
implications to using this in containers.
Ok. Now that I have a reasonable approximation of the 10,000 foot
view now to see how the patches match up.
Eric
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/8] add user mounts to the kernel
Miklos Szeredi [EMAIL PROTECTED] writes:
From: Miklos Szeredi [EMAIL PROTECTED]
Add ownership information to mounts.
A new mount flag, MS_SETUSER is used to make a mount owned by a user.
If this flag is specified, then the owner will be set to the current
real user id and the mount will be marked with the MNT_USER flag. On
remount don't preserve previous owner, and treat MS_SETUSER as for a
new mount. The MS_SETUSER flag is ignored on mount move.
The MNT_USER flag is not copied on any kind of mount cloning:
namespace creation, binding or propagation.
I half agree, and as an initial approximation this works.
Ultimately we should be at the point that for mount propagation
that we copy the owner of the from the owner of our parent mount
at the propagation destination.
Mount propagation semantics are a major pain.
For bind mounts the
cloned mount(s) are set to MNT_USER depending on the MS_SETUSER mount
flag. In all the other cases MNT_USER is always cleared.
For MNT_USER mounts a user=UID option is added to /proc/PID/mounts.
This is compatible with how mount ownership is stored in /etc/mtab.
Ok. While I generally agree with the concept this can be simplified some
more.
Eric
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/fs/namespace.c
===
--- linux.orig/fs/namespace.c 2007-04-20 11:55:02.0 +0200
+++ linux/fs/namespace.c 2007-04-20 11:55:05.0 +0200
@@ -227,6 +227,13 @@ static struct vfsmount *skip_mnt_tree(st
return p;
}
+static void set_mnt_user(struct vfsmount *mnt)
+{
+ BUG_ON(mnt-mnt_flags MNT_USER);
+ mnt-mnt_uid = current-uid;
This should be based on fsuid. Unless I'm completely confused.
I think getting the mount user from current when we do mount
propagation could be a problem. In particular I'm pretty
certain in the mount propagation case the mount should be owned
by the user who owns the destination mount that is above us.
+ mnt-mnt_flags |= MNT_USER;
+}
+
static struct vfsmount *clone_mnt(struct vfsmount *old, struct dentry *root,
int flag)
{
@@ -241,6 +248,11 @@ static struct vfsmount *clone_mnt(struct
mnt-mnt_mountpoint = mnt-mnt_root;
mnt-mnt_parent = mnt;
+ /* don't copy the MNT_USER flag */
+ mnt-mnt_flags = ~MNT_USER;
+ if (flag CL_SETUSER)
+ set_mnt_user(mnt);
+
if (flag CL_SLAVE) {
list_add(mnt-mnt_slave, old-mnt_slave_list);
mnt-mnt_master = old;
@@ -403,6 +415,8 @@ static int show_vfsmnt(struct seq_file *
if (mnt-mnt_flags fs_infop-flag)
seq_puts(m, fs_infop-str);
}
+ if (mnt-mnt_flags MNT_USER)
+ seq_printf(m, ,user=%i, mnt-mnt_uid);
How about making the test if (mnt-mnt_user != root_user)
if (mnt-mnt_sb-s_op-show_options)
err = mnt-mnt_sb-s_op-show_options(m, mnt);
seq_puts(m, 0 0\n);
@@ -920,8 +934,9 @@ static int do_change_type(struct nameida
/*
* do loopback mount.
*/
-static int do_loopback(struct nameidata *nd, char *old_name, int recurse)
+static int do_loopback(struct nameidata *nd, char *old_name, int flags)
{
+ int clone_flags;
struct nameidata old_nd;
struct vfsmount *mnt = NULL;
int err = mount_is_safe(nd);
@@ -941,11 +956,12 @@ static int do_loopback(struct nameidata
if (!check_mnt(nd-mnt) || !check_mnt(old_nd.mnt))
goto out;
+ clone_flags = (flags MS_SETUSER) ? CL_SETUSER : 0;
err = -ENOMEM;
- if (recurse)
- mnt = copy_tree(old_nd.mnt, old_nd.dentry, 0);
+ if (flags MS_REC)
+ mnt = copy_tree(old_nd.mnt, old_nd.dentry, clone_flags);
else
- mnt = clone_mnt(old_nd.mnt, old_nd.dentry, 0);
+ mnt = clone_mnt(old_nd.mnt, old_nd.dentry, clone_flags);
if (!mnt)
goto out;
@@ -987,8 +1003,11 @@ static int do_remount(struct nameidata *
down_write(sb-s_umount);
err = do_remount_sb(sb, flags, data, 0);
- if (!err)
+ if (!err) {
nd-mnt-mnt_flags = mnt_flags;
+ if (flags MS_SETUSER)
+ set_mnt_user(nd-mnt);
+ }
up_write(sb-s_umount);
if (!err)
security_sb_post_remount(nd-mnt, flags, data);
@@ -1093,10 +1112,13 @@ static int do_new_mount(struct nameidata
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- mnt = do_kern_mount(type, flags, name, data);
+ mnt = do_kern_mount(type, flags ~MS_SETUSER, name, data);
if (IS_ERR(mnt))
return PTR_ERR(mnt);
+ if (flags MS_SETUSER)
+ set_mnt_user(mnt);
+
return do_add_mount(mnt, nd, mnt_flags, NULL);
}
@@ -1127,7
Re: [d_path 3/7] Add d_namespace_path() to compute namespace relative pathnames
Hello. I've just returned from ELC2007 and I haven't read all posts in this thread yet, but I want to comment to this function. In AppArmor, we are interested in pathnames relative to the namespace root. This is the same as d_path() except for the root where the search ends. Add a function for computing the namespace-relative path. Yes. You came to the same conclusion as TOMOYO Linux does. http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/fs/realpath.c#L39 TOMOYO Linux uses pathnames relative to the namespace root. You do this using d_path()'s way, but there needs some extensions if you want to use d_namespace_path() for access control/auditing purpose. In Linux, all characters other than NULL can be used in its pathname. This means that you can't assume that whitespaces are delimiters. For example, when you process entries in Access . granted/rejected\n format (where . is a pathname and \n is a carriage return, like Access /bin/ls granted\n), an entry Access /bin/ls granted\nAccess /bin/cat granted\n can be produced if . is /bin/ls granted\nAccess /bin/cat. Processing such entry will produce wrong result. Also, you want wildcards (usually *) when doing pathname comparison, but there are files that contains wildcards (for example, /usr/share/guile/1.6/ice-9/and-let*.scm in CentOS 4.4). You need to escape so that you can tell whether * indicates a literal * or a wildcard. Also, in non-English regions, characters that are out of ASCII printable range are included in its pathname (for example, files created via Samba from Windows client). Some programs can't handle characters that have MSB bit on, so you may want to represent all characters without using MSB bit. It may be OK if you use d_namespace_path() for processing a userland's configuration file, but it is not OK if you use it for processing a kernel's configuration file. The kernel has to be able to handle any characters. So, you may want customized version of d_namespace_path()? - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/8] allow unprivileged umount
Miklos Szeredi [EMAIL PROTECTED] writes:
From: Miklos Szeredi [EMAIL PROTECTED]
The owner doesn't need sysadmin capabilities to call umount().
Similar behavior as umount(8) on mounts having user=UID option in
/etc/mtab. The difference is that umount also checks /etc/fstab,
presumably to exclude another mount on the same mountpoint.
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/fs/namespace.c
===
--- linux.orig/fs/namespace.c 2007-04-20 11:55:05.0 +0200
+++ linux/fs/namespace.c 2007-04-20 11:55:06.0 +0200
@@ -659,6 +659,25 @@ static int do_umount(struct vfsmount *mn
}
/*
+ * umount is permitted for
+ * - sysadmin
+ * - mount owner, if not forced umount
+ */
+static bool permit_umount(struct vfsmount *mnt, int flags)
+{
+ if (capable(CAP_SYS_ADMIN))
+ return true;
+
+ if (!(mnt-mnt_flags MNT_USER))
+ return false;
+
+ if (flags MNT_FORCE)
+ return false;
+
+ return mnt-mnt_uid == current-uid;
+}
I think this should be:
static bool permit_umount(struct vfsmount *mnt, int flags)
{
if ((mnt-mnt_uid != current-fsuid) !capable(CAP_SETUID))
return false;
if ((flags MNT_FORCE) !capable(CAP_SYS_ADMIN))
return false;
return true;
}
I.e.
MNT_USER gone.
compare against fsuid.
Only require setuid for unmounts unless force is specified.
I suspect we can allow MNT_FORCE for non-privileged users
as well if we can trust the filesystem.
+/*
* Now umount can handle mount points as well as block devices.
* This is important for filesystems which use unnamed block devices.
*
@@ -681,7 +700,7 @@ asmlinkage long sys_umount(char __user *
goto dput_and_out;
retval = -EPERM;
- if (!capable(CAP_SYS_ADMIN))
+ if (!permit_umount(nd.mnt, flags))
goto dput_and_out;
retval = do_umount(nd.mnt, flags);
--
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/8] account user mounts
Miklos Szeredi [EMAIL PROTECTED] writes:
From: Miklos Szeredi [EMAIL PROTECTED]
Add sysctl variables for accounting and limiting the number of user
mounts.
The maximum number of user mounts is set to 1024 by default. This
won't in itself enable user mounts, setting a mount to be owned by a
user is first needed
Since each mount has a user can we just make this a per user rlimit?
If we are going to implement a sysctl at this point I think it should
be a global limit that doesn't care if who you are. Even root can
have recursive mounts that attempt to get out of control.
Also currently you are not checking the max_users. It looks like
you do this in a later patch but still it is a little strange to
allow user own mounts and have accounting but to not check the
limit at this state.
Eric
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/include/linux/sysctl.h
===
--- linux.orig/include/linux/sysctl.h 2007-04-20 11:55:02.0 +0200
+++ linux/include/linux/sysctl.h 2007-04-20 11:55:07.0 +0200
@@ -818,6 +818,8 @@ enum
FS_AIO_NR=18, /* current system-wide number of aio requests */
FS_AIO_MAX_NR=19, /* system-wide maximum number of aio requests */
FS_INOTIFY=20, /* inotify submenu */
+ FS_NR_USER_MOUNTS=21, /* int:current number of user mounts */
+ FS_MAX_USER_MOUNTS=22, /* int:maximum number of user mounts */
FS_OCFS2=988, /* ocfs2 */
};
Index: linux/kernel/sysctl.c
===
--- linux.orig/kernel/sysctl.c2007-04-20 11:55:02.0 +0200
+++ linux/kernel/sysctl.c 2007-04-20 11:55:07.0 +0200
@@ -1063,6 +1063,22 @@ static ctl_table fs_table[] = {
#endif
#endif
{
+ .ctl_name = FS_NR_USER_MOUNTS,
+ .procname = nr_user_mounts,
+ .data = nr_user_mounts,
+ .maxlen = sizeof(int),
+ .mode = 0444,
+ .proc_handler = proc_dointvec,
+ },
+ {
+ .ctl_name = FS_MAX_USER_MOUNTS,
+ .procname = max_user_mounts,
+ .data = max_user_mounts,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
.ctl_name = KERN_SETUID_DUMPABLE,
.procname = suid_dumpable,
.data = suid_dumpable,
Index: linux/Documentation/filesystems/proc.txt
===
--- linux.orig/Documentation/filesystems/proc.txt 2007-04-20
11:55:02.0
+0200
+++ linux/Documentation/filesystems/proc.txt 2007-04-20 11:55:07.0
+0200
@@ -923,6 +923,15 @@ reaches aio-max-nr then io_setup will fa
raising aio-max-nr does not result in the pre-allocation or re-sizing
of any kernel data structures.
+nr_user_mounts and max_user_mounts
+--
+
+These represent the number of user mounts and the maximum number of
+user mounts respectively. User mounts may be created by
+unprivileged users. User mounts may also be created with sysadmin
+privileges on behalf of a user, in which case nr_user_mounts may
+exceed max_user_mounts.
+
2.2 /proc/sys/fs/binfmt_misc - Miscellaneous binary formats
---
Index: linux/fs/namespace.c
===
--- linux.orig/fs/namespace.c 2007-04-20 11:55:06.0 +0200
+++ linux/fs/namespace.c 2007-04-20 11:55:07.0 +0200
@@ -39,6 +39,9 @@ static int hash_mask __read_mostly, hash
static struct kmem_cache *mnt_cache __read_mostly;
static struct rw_semaphore namespace_sem;
+int nr_user_mounts;
+int max_user_mounts = 1024;
+
/* /sys/fs */
decl_subsys(fs, NULL, NULL);
EXPORT_SYMBOL_GPL(fs_subsys);
@@ -227,11 +230,30 @@ static struct vfsmount *skip_mnt_tree(st
return p;
}
+static void dec_nr_user_mounts(void)
+{
+ spin_lock(vfsmount_lock);
+ nr_user_mounts--;
+ spin_unlock(vfsmount_lock);
+}
+
static void set_mnt_user(struct vfsmount *mnt)
{
BUG_ON(mnt-mnt_flags MNT_USER);
mnt-mnt_uid = current-uid;
mnt-mnt_flags |= MNT_USER;
+ spin_lock(vfsmount_lock);
+ nr_user_mounts++;
+ spin_unlock(vfsmount_lock);
+}
+
+static void clear_mnt_user(struct vfsmount *mnt)
+{
+ if (mnt-mnt_flags MNT_USER) {
+ mnt-mnt_uid = 0;
+ mnt-mnt_flags = ~MNT_USER;
+ dec_nr_user_mounts();
+ }
}
static struct vfsmount *clone_mnt(struct vfsmount *old, struct dentry *root,
@@ -283,6 +305,7 @@ static inline void __mntput(struct vfsmo
{
struct super_block *sb =
Re: [patch 4/8] propagate error values from clone_mnt
Miklos Szeredi [EMAIL PROTECTED] writes: From: Miklos Szeredi [EMAIL PROTECTED] Allow clone_mnt() to return errors other than ENOMEM. This will be used for returning a different error value when the number of user mounts goes over the limit. Fix copy_tree() to return EPERM for unbindable mounts. Don't propagate further from dup_mnt_ns() as that copy_tree() can only fail with -ENOMEM. At a quick skim this looks ok. Eric - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 5/8] allow unprivileged bind mounts
Miklos Szeredi [EMAIL PROTECTED] writes:
From: Miklos Szeredi [EMAIL PROTECTED]
Allow bind mounts to unprivileged users if the following conditions
are met:
- mountpoint is not a symlink or special file
Why? This sounds like a left over from when we were checking permissions.
- parent mount is owned by the user
- the number of user mounts is below the maximum
Unprivileged mounts imply MS_SETUSER, and will also have the nosuid
and nodev mount flags set.
So in principle I agree, but in detail I disagree.
capable(CAP_SETUID) should be required to leave MNT_NOSUID clear.
capable(CAP_MKNOD) should be required to leave MNT_NODEV clear.
I.e. We should not special case this as a user mount but rather
simply check to see if the user performing the mount has the appropriate
capabilities to allow the flags.
How we properly propagate MNT_NOSUID and MNT_NODEV in the context of a
user id namespace is still a puzzle to me. Because to the user capability
should theoretically at least be namespace local. However until we
get to the user id namespace we don't have that problem.
Eric
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/fs/namespace.c
===
--- linux.orig/fs/namespace.c 2007-04-20 11:55:09.0 +0200
+++ linux/fs/namespace.c 2007-04-20 11:55:10.0 +0200
@@ -237,11 +237,30 @@ static void dec_nr_user_mounts(void)
spin_unlock(vfsmount_lock);
}
-static void set_mnt_user(struct vfsmount *mnt)
+static int reserve_user_mount(void)
+{
+ int err = 0;
+ spin_lock(vfsmount_lock);
+ if (nr_user_mounts = max_user_mounts !capable(CAP_SYS_ADMIN))
+ err = -EPERM;
+ else
+ nr_user_mounts++;
+ spin_unlock(vfsmount_lock);
+ return err;
+}
+
+static void __set_mnt_user(struct vfsmount *mnt)
{
BUG_ON(mnt-mnt_flags MNT_USER);
mnt-mnt_uid = current-uid;
mnt-mnt_flags |= MNT_USER;
+ if (!capable(CAP_SYS_ADMIN))
+ mnt-mnt_flags |= MNT_NOSUID | MNT_NODEV;
+}
+
+static void set_mnt_user(struct vfsmount *mnt)
+{
+ __set_mnt_user(mnt);
spin_lock(vfsmount_lock);
nr_user_mounts++;
spin_unlock(vfsmount_lock);
@@ -260,9 +279,16 @@ static struct vfsmount *clone_mnt(struct
int flag)
{
struct super_block *sb = old-mnt_sb;
- struct vfsmount *mnt = alloc_vfsmnt(old-mnt_devname);
+ struct vfsmount *mnt;
+
+ if (flag CL_SETUSER) {
+ int err = reserve_user_mount();
+ if (err)
+ return ERR_PTR(err);
+ }
+ mnt = alloc_vfsmnt(old-mnt_devname);
if (!mnt)
- return ERR_PTR(-ENOMEM);
+ goto alloc_failed;
mnt-mnt_flags = old-mnt_flags;
atomic_inc(sb-s_active);
@@ -274,7 +300,7 @@ static struct vfsmount *clone_mnt(struct
/* don't copy the MNT_USER flag */
mnt-mnt_flags = ~MNT_USER;
if (flag CL_SETUSER)
- set_mnt_user(mnt);
+ __set_mnt_user(mnt);
if (flag CL_SLAVE) {
list_add(mnt-mnt_slave, old-mnt_slave_list);
@@ -299,6 +325,11 @@ static struct vfsmount *clone_mnt(struct
spin_unlock(vfsmount_lock);
}
return mnt;
+
+ alloc_failed:
+ if (flag CL_SETUSER)
+ dec_nr_user_mounts();
+ return ERR_PTR(-ENOMEM);
}
static inline void __mntput(struct vfsmount *mnt)
@@ -745,22 +776,29 @@ asmlinkage long sys_oldumount(char __use
#endif
-static int mount_is_safe(struct nameidata *nd)
+/*
+ * Conditions for unprivileged mounts are:
+ * - mountpoint is not a symlink or special file
+ * - mountpoint is in a mount owned by the user
+ */
+static bool permit_mount(struct nameidata *nd, int *flags)
{
+ struct inode *inode = nd-dentry-d_inode;
+
if (capable(CAP_SYS_ADMIN))
- return 0;
- return -EPERM;
-#ifdef notyet
- if (S_ISLNK(nd-dentry-d_inode-i_mode))
- return -EPERM;
- if (nd-dentry-d_inode-i_mode S_ISVTX) {
- if (current-uid != nd-dentry-d_inode-i_uid)
- return -EPERM;
- }
- if (vfs_permission(nd, MAY_WRITE))
- return -EPERM;
- return 0;
-#endif
+ return true;
+
+ if (!S_ISDIR(inode-i_mode) !S_ISREG(inode-i_mode))
+ return false;
+
+ if (!(nd-mnt-mnt_flags MNT_USER))
+ return false;
+
+ if (nd-mnt-mnt_uid != current-uid)
+ return false;
+
+ *flags |= MS_SETUSER;
+ return true;
}
Can't this just be:
static bool permit_mount(struct nameidata *nd, uid_t *mnt_uid)
{
*mnt_uid = current-fsuid;
if ((nd-mnt-mnt_uid != current-fsuid) !capable(CAP_SETUID))
return false;
return true;
}
static int lives_below_in_same_fs(struct dentry *d,
Re: [patch 7/8] allow unprivileged mounts
Andrew Morton [EMAIL PROTECTED] writes: On Fri, 20 Apr 2007 12:25:39 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? Also potentially some of the kernel virtual filesystems. /proc should be safe already. If you don't have any kind of backing store this problem gets easier. With unprivileged users allowed to create mounts the utility of kernel functionality exported as filesystems goes up quite a bit. We are not plan9 but this is the last bottleneck in allowing the everything is a filesystem paradigm from being really usable in linux. Eric - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 8/8] allow unprivileged fuse mounts
Miklos Szeredi [EMAIL PROTECTED] writes:
From: Miklos Szeredi [EMAIL PROTECTED]
Use FS_SAFE for fuse fs type, but not for fuseblk.
FUSE was designed from the beginning to be safe for unprivileged
users. This has also been verified in practice over many years. In
addition unprivileged mounts require the parent mount to be owned by
the user, which is more strict than the current userspace policy.
This will enable future installations to remove the suid-root
fusermount utility.
Don't require the user_id= and group_id= options for unprivileged
mounts, but if they are present, verify them for sanity.
Disallow the allow_other option for unprivileged mounts.
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/fs/fuse/inode.c
===
--- linux.orig/fs/fuse/inode.c2007-04-20 11:55:01.0 +0200
+++ linux/fs/fuse/inode.c 2007-04-20 11:55:14.0 +0200
@@ -311,6 +311,19 @@ static int parse_fuse_opt(char *opt, str
d-max_read = ~0;
d-blksize = 512;
+ /*
+ * For unprivileged mounts use current uid/gid. Still allow
+ * user_id and group_id options for compatibility, but
+ * only if they match these values.
+ */
+ if (!capable(CAP_SYS_ADMIN)) {
+ d-user_id = current-uid;
+ d-user_id_present = 1;
+ d-group_id = current-gid;
+ d-group_id_present = 1;
+
+ }
CAP_SETUID is the appropriate capability...
This is not a dimension we have not fully explored.
What is the problem with a user controlled mount having different
uid and gid values.
Yes they map into different users but how is this a problem.
The only problem that I can recall is the historic chown problem
where you could give files to other users and mess up their quotas.
Or is the problem other users writing to this user controlled
filesystem?
Eric
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
2.6.21-rc7 new aops patchset
http://www.kernel.org/pub/linux/kernel/people/npiggin/patches/new-aops/ 2.6.21-rc7-new-aops* New aops patchset against 2.6.21-rc7. I was going to rebase against -mm and submit the patchset, but I ended up doing quite a lot of new filesystem conversions and fixes so I stay with mainline so as to not introduce so much churn. The plan is to next rebase against rc6-mm1 and submit all 47 patches to linux-fsdevel and Andrew Morton to be merged. OK? Should each fs conversion also go to the filesystem specific maintainer/list? I have done some testing, but I would like testing and revuew abd acks from filesystem maintainers. Lots of filesystems I can't or don't know how to mount and test. Well that is a copout, but it is a hassle for me to do so many when presumably maintainers will have a test setup ready to go. And I have tried to minimise the load on maintainers by doing most conversions myself. cc'ed reiserfs lists: could somebody volunteer to do the resierfs conversion? It's too complex for me to do a reasonable job, and reiserfs prepare_write also does weird things with generic_cont_expand which is hard to follow (this could be made much simpler by using the AOP_FLAG_CONT_EXPAND I added there -- can you also look at using that, or let me know why it doesn't work). Changes: - Cut perform_write batch AOP out of this series. It will be back, in a future discussion. Kept the iov_iter stuff because it's nice anyway. - Added patch to inject short or zero length pagecache writes to better exercise filesystems. Another one to poision new and !uptodate buffers. - Kept the old ocfs2 patch from Mark, so I don't have to suck up git-ocfs2.patch which keeps patch size down. I'll use Mark's new patch when rebasing to -mm. - ext4 patch from Badari. - Fixed ext3 data=journal (and the copied code in ext4). - Converted cifs + fixed information leak. - Converted udf seem to have fixed pagecache corruption(?) -- page was marked uptodate when it is not. Also, fixed the silly setup where prepare_write was doing a kmap to be used in commit_write: just do kmap_atomic in write_end. - Fixed some earlier problems with my ext2 multi-page directory handling. - Had a crack at converting all the others. reiserfs is the only one left unconverted! - Tested (under UML, loopback, PAGE_SIZE=4096) with fsx-linux the following: ext2 (bs=1024,4096) ext3 (bs=1024,4096; data=writeback,ordered,journal) minix (v2,3) xfs (bs=1024,4096) fuse (with ssh,passthough fs -- note fsx-linux fails as expected) jfs reiserfs (bs=1024,4096) (unconverted) vfat (bs=1024,4096) tmpfs rd (with ext2) hostfs (it now survives much longer than with the old code!) nfs Nick - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [d_path 3/7] Add d_namespace_path() to compute namespace relative pathnames
On Saturday 21 April 2007 14:57, Tetsuo Handa wrote: So, you may want customized version of d_namespace_path()? No. d_namespace_path() returns valid pathnames, just like d_path() does. Whatever quoting needed can be added to the resulting pathname. Andreas - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
On Apr 21 2007 08:10, Eric W. Biederman wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? Also potentially some of the kernel virtual filesystems. /proc should be safe already. If you don't have any kind of backing store this problem gets easier. tmpfs! Jan -- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
Jan Engelhardt [EMAIL PROTECTED] writes: On Apr 21 2007 08:10, Eric W. Biederman wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? Also potentially some of the kernel virtual filesystems. /proc should be safe already. If you don't have any kind of backing store this problem gets easier. tmpfs! tmpfs is a possible problem because it can consume lots of ram/swap. Which is why it has limits on the amount of space it can consume. Those are set as mount options as I recall. Which means that we would need to do something different with respect to limits before tmpfs could become safe for an untrusted user to mount. Still it's close. Eric - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
On Apr 21 2007 10:57, Eric W. Biederman wrote: tmpfs! tmpfs is a possible problem because it can consume lots of ram/swap. Which is why it has limits on the amount of space it can consume. Users can gobble up all RAM and swap already today. (Unless they are confined into an rlimit, which, in most systems, is not the case.) And in case /dev/shm exists, they can already fill it without running into an rlimit early. Those are set as mount options as I recall. Which means that we would need to do something different with respect to limits before tmpfs could become safe for an untrusted user to mount. Still it's close. Jan -- - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
Andrew Morton [EMAIL PROTECTED] writes: On Fri, 20 Apr 2007 12:25:39 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? If it worked for mount --bind for any fs I could see uses of this. I haven't thought through the security implications though, so it might not work. -Andi - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
Andi Kleen [EMAIL PROTECTED] writes: Andrew Morton [EMAIL PROTECTED] writes: On Fri, 20 Apr 2007 12:25:39 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? If it worked for mount --bind for any fs I could see uses of this. I haven't thought through the security implications though, so it might not work. Binding a directory that you have access to in other was is essentially the same thing as a symlink. So there are no real security implications there. The only problem case I can think of is removal media that you want to remove but someone has made a bind mount to. But that is essentially the same case as opening a file so there are no new real issues. Although our diagnostic tools will likely fall behind for a bit. We handle the security implications by assigning an owner to all mounts and only allowing you to add additional mounts on top of a mount you already own. If you have the right capabilities you can create a mount owned by another user. For a new mount if you don't have the appropriate capabilities nodev and nosuid will be forced. Initial super block creation is a lot more delicate so we need the FS_SAFE flag, to know that the kernel is prepared to deal with the crazy things that a hostile user space is prepared to do. Eric - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/8] allow unprivileged mounts
Andrew Morton wrote: On Fri, 20 Apr 2007 12:25:39 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: Define a new fs flag FS_SAFE, which denotes, that unprivileged mounting of this filesystem may not constitute a security problem. Since most filesystems haven't been designed with unprivileged mounting in mind, a thorough audit is needed before setting this flag. Practically speaking, is there any realistic likelihood that any filesystem apart from FUSE will ever use this? Would it be interesting to support mounting of external file systems (be it USB, NFS or whatever) in a way that automatically forces it to ignore suid and devices (which are already mount time options)? The question I guess is, how much do you gain over a setuid program (hack?) that can handle this? - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] v9fs: don't use primary fid when removing file
v9fs_insert uses v9fs_fid_lookup (which also locks the fid) to get the primary fid associated with the dentry and destroys the v9fs_fid struct after removing the file. If another process called v9fs_fid_lookup on the same dentry, it may wait undefinitely for the fid's lock (as the struct is freed). This patch changes v9fs_remove to use a cloned fid, so the primary fid is not locked and freed. Signed-off-by: Latchesar Ionkov [EMAIL PROTECTED] --- commit ca1a80584fc3211dac158492173467d4f87a27ac tree 787de07bd6d24bdcc9907f90d9085dcd774b2ea4 parent 0f851021c0f91e5073fa89f26b5ac68e23df8e11 author Latchesar Ionkov [EMAIL PROTECTED] Sat, 21 Apr 2007 13:37:15 -0600 committer Latchesar Ionkov [EMAIL PROTECTED] Sat, 21 Apr 2007 13:37:15 -0600 fs/9p/vfs_inode.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index 124a085..b01b0a4 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -415,7 +415,7 @@ static int v9fs_remove(struct inode *dir, struct dentry *file, int rmdir) file_inode = file-d_inode; sb = file_inode-i_sb; v9ses = v9fs_inode2v9ses(file_inode); - v9fid = v9fs_fid_lookup(file); + v9fid = v9fs_fid_clone(file); if(IS_ERR(v9fid)) return PTR_ERR(v9fid); - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
