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:
		
					parent
					
						
							
								7ebab45369
							
						
					
				
			
			
				commit
				
					
						85a258b70d
					
				
			
		
					 1 changed files with 17 additions and 20 deletions
				
			
		|  | @ -1057,42 +1057,40 @@ int ocfs2_ioctl_move_extents(struct file *filp, void __user *argp) | |||
| 
 | ||||
| 	struct inode *inode = file_inode(filp); | ||||
| 	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); | ||||
| 	if (status) | ||||
| 		return status; | ||||
| 
 | ||||
| 	if ((!S_ISREG(inode->i_mode)) || !(filp->f_mode & FMODE_WRITE)) | ||||
| 		goto out; | ||||
| 		goto out_drop; | ||||
| 
 | ||||
| 	if (inode->i_flags & (S_IMMUTABLE|S_APPEND)) { | ||||
| 		status = -EPERM; | ||||
| 		goto out; | ||||
| 		goto out_drop; | ||||
| 	} | ||||
| 
 | ||||
| 	context = kzalloc(sizeof(struct ocfs2_move_extents_context), GFP_NOFS); | ||||
| 	if (!context) { | ||||
| 		status = -ENOMEM; | ||||
| 		mlog_errno(status); | ||||
| 		goto out; | ||||
| 		goto out_drop; | ||||
| 	} | ||||
| 
 | ||||
| 	context->inode = inode; | ||||
| 	context->file = filp; | ||||
| 
 | ||||
| 	if (argp) { | ||||
| 		if (copy_from_user(&range, argp, sizeof(range))) { | ||||
| 			status = -EFAULT; | ||||
| 			goto out; | ||||
| 		} | ||||
| 	} else { | ||||
| 		status = -EINVAL; | ||||
| 		goto out; | ||||
| 	if (copy_from_user(&range, argp, sizeof(range))) { | ||||
| 		status = -EFAULT; | ||||
| 		goto out_free; | ||||
| 	} | ||||
| 
 | ||||
| 	if (range.me_start > i_size_read(inode)) | ||||
| 		goto out; | ||||
| 		goto out_free; | ||||
| 
 | ||||
| 	if (range.me_start + range.me_len > i_size_read(inode)) | ||||
| 			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); | ||||
| 		if (status) | ||||
| 			goto out; | ||||
| 			goto out_copy; | ||||
| 	} | ||||
| 
 | ||||
| 	status = ocfs2_move_extents(context); | ||||
| 	if (status) | ||||
| 		mlog_errno(status); | ||||
| out: | ||||
| out_copy: | ||||
| 	/*
 | ||||
| 	 * movement/defragmentation may end up being partially completed, | ||||
| 	 * that's the reason why we need to return userspace the finished | ||||
| 	 * length and new_offset even if failure happens somewhere. | ||||
| 	 */ | ||||
| 	if (argp) { | ||||
| 		if (copy_to_user(argp, &range, sizeof(range))) | ||||
| 			status = -EFAULT; | ||||
| 	} | ||||
| 	if (copy_to_user(argp, &range, sizeof(range))) | ||||
| 		status = -EFAULT; | ||||
| 
 | ||||
| out_free: | ||||
| 	kfree(context); | ||||
| 
 | ||||
| out_drop: | ||||
| 	mnt_drop_write_file(filp); | ||||
| 
 | ||||
| 	return status; | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Dan Carpenter
				Dan Carpenter