ocfs2: fix error handling in ocfs2_ioctl_move_extents()

Smatch complains that if we hit an error (for example if the file is
immutable) then "range" has uninitialized stack data and we copy it to
the user.

I've re-written the error handling to avoid this problem and make it a
little cleaner as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jie Liu <jeff.liu@oracle.com>
Reviewed-by: Jie Liu <jeff.liu@oracle.com>
Cc: Mark Fasheh <mfasheh@suse.com>
Cc: Joel Becker <jlbec@evilplan.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Dan Carpenter 2013-04-29 15:05:58 -07:00 committed by Linus Torvalds
parent 7ebab45369
commit 85a258b70d
1 changed files with 17 additions and 20 deletions

View File

@ -1057,42 +1057,40 @@ int ocfs2_ioctl_move_extents(struct file *filp, void __user *argp)
struct inode *inode = file_inode(filp); struct inode *inode = file_inode(filp);
struct ocfs2_move_extents range; struct ocfs2_move_extents range;
struct ocfs2_move_extents_context *context = NULL; struct ocfs2_move_extents_context *context;
if (!argp)
return -EINVAL;
status = mnt_want_write_file(filp); status = mnt_want_write_file(filp);
if (status) if (status)
return status; return status;
if ((!S_ISREG(inode->i_mode)) || !(filp->f_mode & FMODE_WRITE)) if ((!S_ISREG(inode->i_mode)) || !(filp->f_mode & FMODE_WRITE))
goto out; goto out_drop;
if (inode->i_flags & (S_IMMUTABLE|S_APPEND)) { if (inode->i_flags & (S_IMMUTABLE|S_APPEND)) {
status = -EPERM; status = -EPERM;
goto out; goto out_drop;
} }
context = kzalloc(sizeof(struct ocfs2_move_extents_context), GFP_NOFS); context = kzalloc(sizeof(struct ocfs2_move_extents_context), GFP_NOFS);
if (!context) { if (!context) {
status = -ENOMEM; status = -ENOMEM;
mlog_errno(status); mlog_errno(status);
goto out; goto out_drop;
} }
context->inode = inode; context->inode = inode;
context->file = filp; context->file = filp;
if (argp) {
if (copy_from_user(&range, argp, sizeof(range))) { if (copy_from_user(&range, argp, sizeof(range))) {
status = -EFAULT; status = -EFAULT;
goto out; goto out_free;
}
} else {
status = -EINVAL;
goto out;
} }
if (range.me_start > i_size_read(inode)) if (range.me_start > i_size_read(inode))
goto out; goto out_free;
if (range.me_start + range.me_len > i_size_read(inode)) if (range.me_start + range.me_len > i_size_read(inode))
range.me_len = i_size_read(inode) - range.me_start; range.me_len = i_size_read(inode) - range.me_start;
@ -1124,25 +1122,24 @@ int ocfs2_ioctl_move_extents(struct file *filp, void __user *argp)
status = ocfs2_validate_and_adjust_move_goal(inode, &range); status = ocfs2_validate_and_adjust_move_goal(inode, &range);
if (status) if (status)
goto out; goto out_copy;
} }
status = ocfs2_move_extents(context); status = ocfs2_move_extents(context);
if (status) if (status)
mlog_errno(status); mlog_errno(status);
out: out_copy:
/* /*
* movement/defragmentation may end up being partially completed, * movement/defragmentation may end up being partially completed,
* that's the reason why we need to return userspace the finished * that's the reason why we need to return userspace the finished
* length and new_offset even if failure happens somewhere. * length and new_offset even if failure happens somewhere.
*/ */
if (argp) {
if (copy_to_user(argp, &range, sizeof(range))) if (copy_to_user(argp, &range, sizeof(range)))
status = -EFAULT; status = -EFAULT;
}
out_free:
kfree(context); kfree(context);
out_drop:
mnt_drop_write_file(filp); mnt_drop_write_file(filp);
return status; return status;