checking vfsmount for r/o flag

From: Dave Hansen
Date: Fri Jun 02 2006 - 10:50:27 EST

Next message: Vivek Goyal: "Re: [Fastboot] [RFC][PATCH] Add missing notifier before crashing"
Previous message: Paolo Ornati: "Re: 2.6.17-rc5-mm2"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Al,

I had first started trying to port some of Herbert Poetzl's patches, but
I realized that there are quite a few things suggested by you and others
back in 2004 that haven't yet been approached. I figured we probably
need to get to these changes first.

http://marc.theaimsgroup.com/?l=linux-kernel&m=107970825119250&w=2

Could you elaborate a bit on how the delayed writes and extra write
request should work?

Basic premise: if you want to through a mount point, you must have first
elevated mnt->writer_count. This effectively encompasses any time a
call is made permission() with the MAY_WRITE flags set.

The following patch takes a stab at auditing all callers to permission()
(and thus ->permission). It sometimes seems a bit high in the layering
(added in quite a few places). This still leaves callers from the
following, but, I figured I'd check that the basic approach is sane for
now:

ecryptfs_permission
vfs_permission
file_permission
user_eo_get
user_eo_set
user_eo_remove
gfs2_repermission
may_create
<- vfs_rename
<- ecryptfs_rename
<- vfs_mknod
<- vfs_mkdir
<- vfs_symlink
<- vfs_link
vfs_rename_dir
nfsd_acceptable
nfsd_permission
nfsd_permission
xattr_permission

I'm a bit confused by some messages from 2004. It seems
vfs_permission() has changed function a bit since then:

http://marc.theaimsgroup.com/?l=linux-kernel&m=107961332908524&w=2

-- Dave

--- linux.orig/fs/namei.c~convert-permission-to-file-and-vfs 2006-06-02 07:40:33.000000000 -0700
+++ linux/fs/namei.c 2006-06-02 07:40:34.000000000 -0700
@@ -1461,22 +1461,29 @@
int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
struct nameidata *nd)
{
- int error = may_create(dir, dentry, nd);
+ int error;

+ error = mnt_want_write(nd.mnt);
if (error)
return error;

+ error = may_create(dir, dentry, nd);
+ if (error)
+ goto out;
+
if (!dir->i_op || !dir->i_op->create)
return -EACCES; /* shouldn't it be ENOSYS? */
mode &= S_IALLUGO;
mode |= S_IFREG;
error = security_inode_create(dir, dentry, mode);
if (error)
- return error;
+ goto out;
DQUOT_INIT(dir);
error = dir->i_op->create(dir, dentry, mode, nd);
if (!error)
fsnotify_create(dir, dentry);
+out:
+ mnt_put_write(nd.mnt);
return error;
}

@@ -2007,13 +2014,21 @@
error = -EBUSY;
goto exit1;
}
+ error = -EROFS;
+ if (nd_is_readonly(&nd))
+ goto exit1;
mutex_lock(&nd.dentry->d_inode->i_mutex);
dentry = lookup_hash(&nd);
error = PTR_ERR(dentry);
- if (!IS_ERR(dentry)) {
- error = vfs_rmdir(nd.dentry->d_inode, dentry);
- dput(dentry);
- }
+ if (IS_ERR(dentry))
+ goto exit2;
+ if (!mnt_want_write(nd.mnt))
+ goto exit3;
+ error = vfs_rmdir(nd.dentry->d_inode, dentry);
+ mnt_drop_write(nd.mnt);
+exit3:
+ dput(dentry);
+exit2:
mutex_unlock(&nd.dentry->d_inode->i_mutex);
exit1:
path_release(&nd);
@@ -2081,6 +2096,8 @@
error = -EISDIR;
if (nd.last_type != LAST_NORM)
goto exit1;
+ if (nd_is_readonly(&nd))
+ goto exit1;
mutex_lock(&nd.dentry->d_inode->i_mutex);
dentry = lookup_hash(&nd);
error = PTR_ERR(dentry);
@@ -2091,7 +2108,11 @@
inode = dentry->d_inode;
if (inode)
atomic_inc(&inode->i_count);
+ error = mnt_want_write(nd.mnt);
+ if (error)
+ goto exit2;
error = vfs_unlink(nd.dentry->d_inode, dentry);
+ mnt_put_write(nd.mnt);
exit2:
dput(dentry);
}
@@ -2456,16 +2477,21 @@
if (newnd.last_type != LAST_NORM)
goto exit2;

+ if (!mnt_want_write(oldnd.mnt))
+ goto exit2;
+ if (!mnt_want_write(newnd.mnt))
+ goto exit3;
+
trap = lock_rename(new_dir, old_dir);

old_dentry = lookup_hash(&oldnd);
error = PTR_ERR(old_dentry);
if (IS_ERR(old_dentry))
- goto exit3;
+ goto exit4;
/* source must exist */
error = -ENOENT;
if (!old_dentry->d_inode)
- goto exit4;
+ goto exit5;
/* unless the source is a directory trailing slashes give -ENOTDIR */
if (!S_ISDIR(old_dentry->d_inode->i_mode)) {
error = -ENOTDIR;
@@ -2477,24 +2503,27 @@
/* source should not be ancestor of target */
error = -EINVAL;
if (old_dentry == trap)
- goto exit4;
+ goto exit5;
new_dentry = lookup_hash(&newnd);
error = PTR_ERR(new_dentry);
if (IS_ERR(new_dentry))
- goto exit4;
+ goto exit5;
/* target should not be an ancestor of source */
error = -ENOTEMPTY;
if (new_dentry == trap)
- goto exit5;
+ goto exit6;

error = vfs_rename(old_dir->d_inode, old_dentry,
new_dir->d_inode, new_dentry);
-exit5:
+exit6:
dput(new_dentry);
-exit4:
+exit5:
dput(old_dentry);
-exit3:
+exit4:
+ mnt_drop_write(newnd.mnt);
unlock_rename(new_dir, old_dir);
+exit3:
+ mnt_drop_write(oldnd.mnt);
exit2:
path_release(&newnd);
exit1:
--- linux.orig/fs/namespace.c~convert-permission-to-file-and-vfs 2006-06-02 07:40:33.000000000 -0700
+++ linux/fs/namespace.c 2006-06-02 07:40:33.000000000 -0700
@@ -66,6 +66,7 @@
if (mnt) {
memset(mnt, 0, sizeof(struct vfsmount));
atomic_set(&mnt->mnt_count, 1);
+ /* atomic_set(&mnt->writer_count, 0); */
INIT_LIST_HEAD(&mnt->mnt_hash);
INIT_LIST_HEAD(&mnt->mnt_child);
INIT_LIST_HEAD(&mnt->mnt_mounts);
--- linux.orig/include/linux/mount.h~convert-permission-to-file-and-vfs 2006-06-02 07:40:33.000000000 -0700
+++ linux/include/linux/mount.h 2006-06-02 07:40:33.000000000 -0700
@@ -38,6 +38,7 @@
struct list_head mnt_mounts; /* list of children, anchored here */
struct list_head mnt_child; /* and going through their mnt_child */
atomic_t mnt_count;
+ atomic_t writer_count;
int mnt_flags;
int mnt_expiry_mark; /* true if marked for expiry */
char *mnt_devname; /* Name of device e.g. /dev/dsk/hda1 */
@@ -58,6 +59,26 @@
return mnt;
}

+static inline int mnt_readonly(struct vfsmount *mnt)
+{
+ if (mnt->mnt_sb->s_flags & MS_RDONLY)
+ return -EROFS;
+ return 0;
+}
+
+static inline int mnt_want_write(struct vfsmount *mnt)
+{
+ if (mnt_readonly(mnt))
+ return -EROFS;
+ atomic_inc(&mnt->mnt_writers);
+ return 0;
+}
+
+static inline int mnt_drop_write(struct vfsmount *mnt)
+{
+ atomic_dec(&mnt->mnt_writers);
+}
+
extern void mntput_no_expire(struct vfsmount *mnt);
extern void mnt_pin(struct vfsmount *mnt);
extern void mnt_unpin(struct vfsmount *mnt);
--- linux.orig/ipc/mqueue.c~elevate-writers-vfs_unlink 2006-06-02 07:40:34.000000000 -0700
+++ linux/ipc/mqueue.c 2006-06-02 07:40:34.000000000 -0700
@@ -738,8 +738,11 @@
inode = dentry->d_inode;
if (inode)
atomic_inc(&inode->i_count);
-
+ err = mnt_want_write(mqueue_mnt);
+ if (err)
+ goto out_err;
err = vfs_unlink(dentry->d_parent->d_inode, dentry);
+ mnt_put_write(mqueue_mnt);
out_err:
dput(dentry);

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

Next message: Vivek Goyal: "Re: [Fastboot] [RFC][PATCH] Add missing notifier before crashing"
Previous message: Paolo Ornati: "Re: 2.6.17-rc5-mm2"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]