forge/linux-pinenote
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ocfs2: fix error handling in ocfs2_ioctl_move_extents()

Browse source 
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
commit 85a258b70d
1 changed files with 17 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

29
fs/ocfs2/move_extents.c
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;