linux-ext4 - Re: [PATCH] Prevent creation of files larger than RLIMIT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 3 May 2010 09:53:44 +0530
From:	Nikanth Karthikesan <knikanth@...e.de>
To:	"Amit K. Arora" <aarora@...ux.vnet.ibm.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, coly.li@...e.de,
	Nick Piggin <npiggin@...e.de>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, "Theodore Ts'o" <tytso@....edu>,
	Andreas Dilger <adilger@....com>, linux-ext4@...r.kernel.org,
	Eelis <opensuse.org@...tacts.eelis.net>,
	Amit Arora <aarora@...ibm.com>,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH] Prevent creation of files larger than RLIMIT_FSIZE using fallocate

On Saturday 01 May 2010 12:34:26 Amit K. Arora wrote:
> On Fri, Apr 30, 2010 at 02:33:19PM -0700, Andrew Morton wrote:
> > (Amit Arora <aarora@...ibm.com> wrote fallocate.  cc added)
> 
> Thanks for adding me to CC.
> 
> > On Thu, 29 Apr 2010 10:14:06 +0530
> >
> > Nikanth Karthikesan <knikanth@...e.de> wrote:
> > > Here is an updated patch that takes the i_mutex and calls
> > > inode_newsize_ok() only for regular files.
> >
> > err, no.  It's taking i_lock where it meant to take i_mutex.
> >
> > > Thanks
> > > Nikanth
> > >
> > > +	if (S_ISREG(inode->i_mode)) {
> > > +		spin_lock(&inode->i_lock);
> > > +		ret = inode_newsize_ok(inode, (offset + len));
> > > +		spin_unlock(&inode->i_lock);
> > > +		if (ret)
> > > +			return ret;
> > > +	} else if (S_ISDIR(inode->i_mode)) {
> > > +		/*
> > > +		 * Let individual file system decide if it supports
> > > +		 * preallocation for directories or not.
> > > +		 */
> > > +		if (offset > inode->i_sb->s_maxbytes)
> > > +			return -EFBIG;
> > > +	} else
> > > +		return -ENODEV;
> > > +
> > >  	if (!inode->i_op->fallocate)
> > >  		return -EOPNOTSUPP;
> >
> > Also, there doesn't seem to be much point in doing
> >
> > 	mutex_lock(i_mutex);
> > 	if (some_condition)
> > 		bale out
> > 	mutex_unlock(i_mutex);
> >
> > 	<stuff>
> >
> > because `some_condition' can now become true before or during the
> > execution of `stuff'.
> >
> > IOW, it's racy.
> 

oh, yes. :(

> Agreed. How about doing this check in the filesystem specific fallocate
> inode routines instead ? For example, in ext4 we could do :
> 

I guess, calling the filesystem specific fallocate with the lock held would 
create lock ordering problems? If so, this might be the only way. But it would 
be better to document at the call site, that the callee should check for 
RLIMIT_FSIZE.

Thanks
Nikanth

> diff -Nuarp linux-2.6.org/fs/ext4/extents.c linux-2.6.new/fs/ext4/extents.c
> --- linux-2.6.org/fs/ext4/extents.c	2010-05-01 12:16:07.000000000 +0530
> +++ linux-2.6.new/fs/ext4/extents.c	2010-05-01 12:17:37.000000000 +0530
> @@ -3672,6 +3672,11 @@ long ext4_fallocate(struct inode *inode,
>  	 */
>  	credits = ext4_chunk_trans_blocks(inode, max_blocks);
>  	mutex_lock(&inode->i_mutex);
> +	ret = inode_newsize_ok(inode, (offset + len));
> +	if (ret) {
> +		mutex_unlock(&inode->i_mutex);
> +		return ret;
> +	}
>  retry:
>  	while (ret >= 0 && ret < max_blocks) {
>  		block = block + ret;
> 
> 
> Similarly for ocfs2, btrfs and xfs..
> 
> --
> Regards,
> Amit Arora
> 
--
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