On 04/28/2010 09:24 PM, Nikanth Karthikesan Wrote:
> Prevent creation of files larger than RLIMIT_FSIZE using fallocate.
>
> Currently using posix_fallocate one can bypass an RLIMIT_FSIZE limit
> and create a file larger than the limit. Add a check for new size in
> the fallocate system call.
>
> File-systems supporting fallocate such as ext4 are affected by this
> bug.
>
> Signed-off-by: Nikanth Karthikesan <knikanth@xxxxxxx>
> Reported-by: Eelis - <opensuse.org@xxxxxxxxxxxxxxxxxx>
>
> ---
>
> diff --git a/fs/open.c b/fs/open.c
> index 74e5cd9..95ce069 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -412,10 +412,14 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
> if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> return -ENODEV;
>
> - /* Check for wrap through zero too */
> - if (((offset + len) > inode->i_sb->s_maxbytes) || ((offset + len) < 0))
> + /* Check for wrap through zero */
> + if (offset+len < 0)
> return -EFBIG;
>
> + ret = inode_newsize_ok(inode, (offset + len));
> + if (ret)
> + return ret;
> +
> if (!inode->i_op->fallocate)
> return -EOPNOTSUPP;
>
Hi Nikanth,
>From definition of inode_newsize_ok(), it says,
64 /**
65 * inode_newsize_ok - may this inode be truncated to a given size
66 * @inode: the inode to be truncated
67 * @offset: the new size to assign to the inode
68 * @Returns: 0 on success, -ve errno on failure
69 *
70 * inode_newsize_ok will check filesystem limits and ulimits to check that the
71 * new inode size is within limits. inode_newsize_ok will also send SIGXFSZ
72 * when necessary. Caller must not proceed with inode size change if failure is
73 * returned. @inode must be a file (not directory), with appropriate
74 * permissions to allow truncate (inode_newsize_ok does NOT check these
75 * conditions).
76 *
77 * inode_newsize_ok must be called with i_mutex held.
78 */
In execution path of do_fallocate(), it seems no i_mutex held, and inode might be directory. Using inode_newsize_ok()
might be confused ?
How about this one ?
fs/open.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index 74e5cd9..24d75a4 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -383,6 +383,7 @@ SYSCALL_ALIAS(sys_ftruncate64, SyS_ftruncate64);
int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
{
struct inode *inode = file->f_path.dentry->d_inode;
+ unsigned long limit;
long ret;
if (offset < 0 || len <= 0)
@@ -412,10 +413,19 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
return -ENODEV;
- /* Check for wrap through zero too */
- if (((offset + len) > inode->i_sb->s_maxbytes) || ((offset + len) < 0))
+ /* Check for wrap through zero */
+ if ((offset + len) < 0)
return -EFBIG;
+ /* Check for rlimit */
+ if ((offset + len) > inode->i_size) {
+ limit = rlimit(RLIMIT_FSIZE);
+ if (limit != RLIM_INFINITY && (offset + len) > limit)
+ return -EFBIG;
+ if ((offset + len) > inode->i_sb->s_maxbytes)
+ return -EFBIG;
+ }
+
if (!inode->i_op->fallocate)
return -EOPNOTSUPP;
Something I don't understand here is, why no i_mutex hold here ?
--
Coly Li
SuSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
