Message ID | 20120106010011.GA9028@dztty |
---|---|
State | Not Applicable, archived |
Headers | show |
On Fri 06-01-12 02:00:11, Djalal Harouni wrote: > On Thu, Jan 05, 2012 at 12:42:05PM +0100, Jan Kara wrote: > > On Thu 05-01-12 01:40:09, Djalal Harouni wrote: > > > Only the reiserfs and ext{2,3,4} filesystems support this ioctl. The reiserfs > > > do not use mutexes at all, even in the REISERFS_IOC_SETFLAGS ioctl which will > > > test and set _all_ the possible values of the i_flags field. > > > Perhaps I should also send a patch for this ? > > Yes, possibly reiserfs should use i_mutex for that ioctl. > For the reiserfs I've missed some locks. > > It seems that reiserfs uses its own 'reiserfs_sb_info->lock' (reiserfs > super block data) to serialize writers in all the ioctl, even the GET > (readers) ioctl operations. But I also know that the VFS layer uses i_mutex > to protect inode changes, so ? I'm not sure, If I can test it I'll send a > patch. > > > > And perhaps ext2 should also be updated. > > Sure. Send a patch my way when you have it. > Here's the tested ext2 patch, thanks. Thanks I've taken it into my tree. Honza > -------- > > From: Djalal Harouni <tixxdz@opendz.org> > > ext2: protect inode changes in the SETVERSION and SETFLAGS ioctls > > Unlock mutex after i_flags and i_ctime updates in the EXT2_IOC_SETFLAGS > ioctl. > > Use i_mutex in the EXT2_IOC_SETVERSION ioctl to protect i_ctime and > i_generation updates and make the ioctl consistent since i_mutex is > also used in other places to protect timestamps and inode changes. > > Cc: Andreas Dilger <adilger.kernel@dilger.ca> > Cc: Jan Kara <jack@suse.cz> > Signed-off-by: Djalal Harouni <tixxdz@opendz.org> > --- > fs/ext2/ioctl.c | 22 ++++++++++++++++------ > 1 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c > index f81e250..b7f931f 100644 > --- a/fs/ext2/ioctl.c > +++ b/fs/ext2/ioctl.c > @@ -77,10 +77,11 @@ long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > flags = flags & EXT2_FL_USER_MODIFIABLE; > flags |= oldflags & ~EXT2_FL_USER_MODIFIABLE; > ei->i_flags = flags; > - mutex_unlock(&inode->i_mutex); > > ext2_set_inode_flags(inode); > inode->i_ctime = CURRENT_TIME_SEC; > + mutex_unlock(&inode->i_mutex); > + > mark_inode_dirty(inode); > setflags_out: > mnt_drop_write(filp->f_path.mnt); > @@ -88,20 +89,29 @@ setflags_out: > } > case EXT2_IOC_GETVERSION: > return put_user(inode->i_generation, (int __user *) arg); > - case EXT2_IOC_SETVERSION: > + case EXT2_IOC_SETVERSION: { > + __u32 generation; > + > if (!inode_owner_or_capable(inode)) > return -EPERM; > ret = mnt_want_write(filp->f_path.mnt); > if (ret) > return ret; > - if (get_user(inode->i_generation, (int __user *) arg)) { > + if (get_user(generation, (int __user *) arg)) { > ret = -EFAULT; > - } else { > - inode->i_ctime = CURRENT_TIME_SEC; > - mark_inode_dirty(inode); > + goto setversion_out; > } > + > + mutex_lock(&inode->i_mutex); > + inode->i_ctime = CURRENT_TIME_SEC; > + inode->i_generation = generation; > + mutex_unlock(&inode->i_mutex); > + > + mark_inode_dirty(inode); > +setversion_out: > mnt_drop_write(filp->f_path.mnt); > return ret; > + } > case EXT2_IOC_GETRSVSZ: > if (test_opt(inode->i_sb, RESERVATION) > && S_ISREG(inode->i_mode) > -- > 1.7.1
diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c index f81e250..b7f931f 100644 --- a/fs/ext2/ioctl.c +++ b/fs/ext2/ioctl.c @@ -77,10 +77,11 @@ long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) flags = flags & EXT2_FL_USER_MODIFIABLE; flags |= oldflags & ~EXT2_FL_USER_MODIFIABLE; ei->i_flags = flags; - mutex_unlock(&inode->i_mutex); ext2_set_inode_flags(inode); inode->i_ctime = CURRENT_TIME_SEC; + mutex_unlock(&inode->i_mutex); + mark_inode_dirty(inode); setflags_out: mnt_drop_write(filp->f_path.mnt); @@ -88,20 +89,29 @@ setflags_out: } case EXT2_IOC_GETVERSION: return put_user(inode->i_generation, (int __user *) arg); - case EXT2_IOC_SETVERSION: + case EXT2_IOC_SETVERSION: { + __u32 generation; + if (!inode_owner_or_capable(inode)) return -EPERM; ret = mnt_want_write(filp->f_path.mnt); if (ret) return ret; - if (get_user(inode->i_generation, (int __user *) arg)) { + if (get_user(generation, (int __user *) arg)) { ret = -EFAULT; - } else { - inode->i_ctime = CURRENT_TIME_SEC; - mark_inode_dirty(inode); + goto setversion_out; } + + mutex_lock(&inode->i_mutex); + inode->i_ctime = CURRENT_TIME_SEC; + inode->i_generation = generation; + mutex_unlock(&inode->i_mutex); + + mark_inode_dirty(inode); +setversion_out: mnt_drop_write(filp->f_path.mnt); return ret; + } case EXT2_IOC_GETRSVSZ: if (test_opt(inode->i_sb, RESERVATION) && S_ISREG(inode->i_mode)