lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120109150338.GB18898@quack.suse.cz> Date: Mon, 9 Jan 2012 16:03:38 +0100 From: Jan Kara <jack@...e.cz> To: Djalal Harouni <tixxdz@...ndz.org> Cc: Jan Kara <jack@...e.cz>, Andreas Dilger <adilger.kernel@...ger.ca>, Andrew Morton <akpm@...ux-foundation.org>, "Darrick J. Wong" <djwong@...ibm.com>, Theodore Ts'o <tytso@....edu>, Yongqiang Yang <xiaoqiangnk@...il.com>, ext4 development <linux-ext4@...r.kernel.org>, linux-kernel Mailing List <linux-kernel@...r.kernel.org>, Al Viro <viro@...iv.linux.org.uk> Subject: Re: [PATCH] fs/ext{3,4}: fix potential race when setversion ioctl updates inode 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@...ndz.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@...ger.ca> > Cc: Jan Kara <jack@...e.cz> > Signed-off-by: Djalal Harouni <tixxdz@...ndz.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 -- Jan Kara <jack@...e.cz> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists